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

Lot Number not included in Project Runs #1152

Open
JoarGjersund opened this issue Sep 17, 2020 · 12 comments · May be fixed by #1153
Open

Lot Number not included in Project Runs #1152

JoarGjersund opened this issue Sep 17, 2020 · 12 comments · May be fixed by #1153

Comments

@JoarGjersund
Copy link

JoarGjersund commented Sep 17, 2020

How to reproduce:

  1. Create a new project under "edit > projects"
  2. Add a part and fill out the field "Lot number" with "12345" and click "Save project".
  3. Generate a project report under "view > Project Reports", select the project and set Qty to 1 and click Create Report.
  4. Click "Remove parts from stock"
  5. Go to "View > Project Runs" and view the last run.

What happens: "Lot number field is empty"
What is expected: Lot number should be "12345".

Solve this and get a bounty

@christianlupus
Copy link
Collaborator

I can confirm this issue.

I just created a HAR dump from firefox to have this documented. During massRemoveStock the lot number seems not be transfered, if I see it correctly. Thus it needs to be implemented in the project run (to be present when clicking on "remove parts from stock".

ed-commits added a commit to ed-commits/PartKeepr that referenced this issue Sep 26, 2020
Fix partkeepr#1152 by extracting the project part rather than the raw part.
@ed-commits ed-commits linked a pull request Sep 26, 2020 that will close this issue
@ed-commits
Copy link

Hello! I saw this issue on bountysource so I had a look at the code.

I was able to reproduce the bug. By inspecting http://partkeepr.local/api/project_run_parts/1 I saw that lotNumber is set in the part inside project, but not set in the toplevel part. Based on this I think it's just a display issue rather than the lotNumber value disappearing, indeed lotNumber is copied over inside massRemoveStockAction of src/PartKeepr/PartBundle/Controller/PartController.php.

How does the following fix look? #1153

@dromer
Copy link
Contributor

dromer commented Oct 1, 2020

Would be nice to get confirmation that your PR fixes this issue completely!

@JoarGjersund
Copy link
Author

I tried to implement the changes but it did not seem to have any effect. I only implemented the changes and did a re-run of the /setup page. Do I need to do run any composer commands as well?

@ed-commits
Copy link

I think the cache of the frontend javascript files must be deleted for this change to apply. I just deleted them then reran setup but later I found these commands which may work better https://wiki.partkeepr.org/wiki/Running_PartKeepr_from_GIT#Console_commands

@JoarGjersund
Copy link
Author

JoarGjersund commented Oct 6, 2020

@ed-commits I am a total noob with this environment, so sorry for my stupid questions. But by deleting the frontend javascript cache do you mean running the rm -rf app/cache/* command ? I have tried this, before runnin the setup, but I am still not able to make it include the lot number in the project runs. Can you provide me with the exact procedure you used to verify the patch so I can reproduce?

@ed-commits
Copy link

I did that, also maybe try rm -rf web/js/compiled and rm -rf web/js/packages/extjs6 too. then rerun setup. then the change should apply.

@JoarGjersund
Copy link
Author

@ed-commits I have now tried this as well, and I am unfortuently not able see that the change makes any difference. Are someone else able to confirm that this solves the issue?

@JoarGjersund
Copy link
Author

JoarGjersund commented Oct 12, 2020

I can now confirm that #1153 fixes this issue. Seems like the cache somewhere was reset after a while. However removing compiled and extjs6 is not reccomended. It will brick the whole system, making it stuck in in loading page, and setup page was be left blank. This issue can be closed once the fix is merged to master.

edit: Obs, I now see that the lot number in the project Run will change if the Project changes (under edit-> projects). This is not correct. The Lot number should not be possible to change after a project run. So not linked directly to the project, if that makes sense.

@JoarGjersund
Copy link
Author

JoarGjersund commented Oct 12, 2020

I think the issue is indeed somewhere in the massremovestock action
here:

$projectRunPart->setLotNumber($removal->lotNumber);

Or rather, that the $removal->lotNumber field is empty. So more specifially somewhere in the json request

$removals = json_decode($request->get('removals'));

Which I guess is set somewhere here:

lotNumber: item.projectParts().getFieldValues("lotNumber").join(", "),

@ghost-from-the-past
Copy link

Just in case is useful,
I notice the there is some kind of logic problem with the button "Save Project" when importing part into the project.
Once all the process done (read of CSV file, click "Execute Import") successful to add the list of parts to the project, the table ProjectPart is correctly fulfilled, even closing the import window continues to be correct but when you click the "Save Project" button it discharge/revert the changes (?)
at the end of this page is explained how the users circumvent this
https://readthedocs.web.cern.ch/display/PARTK/07a+-+Creating+Projects+and+BOM+Imports

the mention of the "save project" at the beginning of this issue and the behavior described seem similar to what I described.
Regards

@ghost-from-the-past
Copy link

Dear JoarGjersund and ed-commits
If I understood correctly when you run http://localhost/web/app_dev.php
https://readthedocs.web.cern.ch/display/PARTK/Setup+for+Debug+and+Verbose+mode
you don't care about the cache, what you execute is done directly.
Regards

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants