Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: link old BOMs in BOM Creator #38526

Closed
wants to merge 12 commits into from

Conversation

SvbZ3r0
Copy link
Contributor

@SvbZ3r0 SvbZ3r0 commented Dec 3, 2023

Foundational work for #38438
@rohitwaghchaure your input is extremely valuable

Pending work:

  • Disallow editing sub nodes where data has been pulled from an existing BOM
  • Qty edits on a node should propagate downwards if data is pulled from an exisiting BOM
  • Validation/checkpoints to ensure direct edits to the Item table do not clash with data from existing BOMs

Nice to have:

  • Allow choosing which BOM to use instead of automatically using the default BOM
  • Play nice with BOMs that have Scrap Material (feature request for BOM Creator)
  • Play nice with Items that have Subcontracting BOMs and with BOMs that have Operations (re: feat: track Semi-finished goods (including subcontracted items) against Job Cards #38341)
  • RM table is read only if BOM is selected Hide Raw Materials table in Add Sub Assembly dialog if Use Existing BOM is selected and there is a valid BOM present

@github-actions github-actions bot added the needs-tests This PR needs automated unit-tests. label Dec 3, 2023
@SvbZ3r0 SvbZ3r0 force-pushed the link-old-bom-in-creator branch from edb1dc3 to 542047e Compare December 3, 2023 07:12
@SvbZ3r0 SvbZ3r0 force-pushed the link-old-bom-in-creator branch from 542047e to ff5e74f Compare December 3, 2023 07:12
Copy link

codecov bot commented Dec 3, 2023

Codecov Report

Attention: 14 lines in your changes are missing coverage. Please review.

Comparison is base (33b631e) 60.03% compared to head (49f8f91) 60.14%.
Report is 171 commits behind head on develop.

❗ Current head 49f8f91 differs from pull request most recent head cc0f4a9. Consider uploading reports for the commit cc0f4a9 to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #38526      +/-   ##
===========================================
+ Coverage    60.03%   60.14%   +0.11%     
===========================================
  Files          761      759       -2     
  Lines        70122    69508     -614     
===========================================
- Hits         42099    41808     -291     
+ Misses       28023    27700     -323     
Files Coverage Δ
...uring/doctype/bom_creator_item/bom_creator_item.py 15.15% <0.00%> (-0.98%) ⬇️
...t/manufacturing/doctype/bom_creator/bom_creator.py 70.03% <45.45%> (-2.54%) ⬇️

... and 63 files with indirect coverage changes

@rohitwaghchaure
Copy link
Collaborator

@SvbZ3r0 Please add GIF or screenshots

@SvbZ3r0
Copy link
Contributor Author

SvbZ3r0 commented Dec 3, 2023

bomcreator1

bomcreator2

@SvbZ3r0
Copy link
Contributor Author

SvbZ3r0 commented Dec 3, 2023

bomcreatorqtyupdate

Btw, what do you guys use for sreen captures?

@rohitwaghchaure
Copy link
Collaborator

@SvbZ3r0 Thanks for sharing the GIFs

My feedback is as follows

  1. Use existing BOM :- Instead of this checkbox, you can add BOM field and on selection of the BOM fetch the raw materials from the respective BOM. Don't allow to change the raw materials or their quantities.
  2. Qty edits on a node should propagate downwards: I think this should be configurable, what if user want to edit the qty of parent item bu don't want to propagate downwards?

@SvbZ3r0
Copy link
Contributor Author

SvbZ3r0 commented Dec 4, 2023

Thanks for the feedback.

  1. That is how i want to do it too, but I don't know how. Is there some other place in the repo where something similar is done? Specifically, i want to know how to work with Dialog fields; adding event listeners, setting filters, refreshing the fields, etc.

  2. I will do that. Any pointers on how to make it optional?

@SvbZ3r0
Copy link
Contributor Author

SvbZ3r0 commented Dec 7, 2023

choosebom.mp4

@rohitwaghchaure feedback implemented

@SvbZ3r0 SvbZ3r0 force-pushed the link-old-bom-in-creator branch from d0bbc5c to a50d67c Compare December 9, 2023 02:22
@SvbZ3r0 SvbZ3r0 force-pushed the link-old-bom-in-creator branch from a50d67c to 32043de Compare December 9, 2023 02:25
@SvbZ3r0
Copy link
Contributor Author

SvbZ3r0 commented Dec 9, 2023

itemsfieldreadonly.mp4
itemsfieldreadonly2.mp4

@SvbZ3r0 SvbZ3r0 marked this pull request as ready for review December 9, 2023 02:50
@rohitwaghchaure rohitwaghchaure self-assigned this Dec 12, 2023
@SvbZ3r0
Copy link
Contributor Author

SvbZ3r0 commented Dec 22, 2023

Hi @rohitwaghchaure ! Anything holding this up? I might be able to work on it some more during the holiday if needed.

Copy link

stale bot commented Jan 12, 2024

This pull request has been automatically marked as inactive because it has not had recent activity. It will be closed within 3 days if no further activity occurs, but it only takes a comment to keep a contribution alive :) Also, even if it is closed, you can always reopen the PR when you're ready. Thank you for contributing.

@stale stale bot added the inactive label Jan 12, 2024
@SvbZ3r0
Copy link
Contributor Author

SvbZ3r0 commented Jan 12, 2024

Boop

@stale stale bot closed this Jan 16, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 31, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
inactive needs-tests This PR needs automated unit-tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants