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

[11.0][ADD] Scrap reason code module #770

Merged
merged 1 commit into from
Apr 1, 2020

Conversation

bodedra
Copy link
Member

@bodedra bodedra commented Nov 26, 2019

Usage

In the Inventory module, open the Configuration menu and select Scrap Reason Codes.
Create the required scrap reason codes. Under Operations, select Scrap. Click the
create button to create a new scrap order. You will see a reason code field on the
scrap form which will allow you to select any of the scrap codes you created previously.

@OCA-git-bot
Copy link
Contributor

Hi @bodedra,
some modules you are maintaining are being modified, check this out!

@bodedra bodedra marked this pull request as ready for review November 26, 2019 16:51
@bodedra
Copy link
Member Author

bodedra commented Nov 27, 2019

@hveficent @aheficent @lreficent @jbeficent @max3903

Would you please help me.

Test case is failed in stock_request or stock_request_analytic module.

Firefox_Screenshot_2019-11-27T08-06-10 411Z

@bodedra
Copy link
Member Author

bodedra commented Nov 27, 2019

Are you available for review?

@b-kannan @osimallen @smangukiya @gavindav @diverfr @MDodoo @guimarc-br

Test case are green for this PR.

Firefox_Screenshot_2019-11-27T08-03-04 972Z

@bodedra bodedra force-pushed the 11.0-scrap_reason_code branch 3 times, most recently from 1dfd11f to 412bb40 Compare November 27, 2019 10:26
@bodedra
Copy link
Member Author

bodedra commented Nov 27, 2019

ci is 🍏

@HviorForgeFlow
Copy link
Member

@bodedra all I can see is that stock_request_user is not available to read stock.warehouse0 record.
Can't see why

@bodedra
Copy link
Member Author

bodedra commented Nov 27, 2019

@bodedra all I can see is that stock_request_user is not available to read stock.warehouse0 record.
Can't see why

@hveficent Agree. I verified that read access right is already set. Here is ref https://github.com/OCA/stock-logistics-warehouse/blob/11.0/stock_request/security/ir.model.access.csv#L14

@bodedra
Copy link
Member Author

bodedra commented Nov 28, 2019

@rousseldenis Is there any suggestion for fix #770 (comment)

Read access right already for warehouse. https://github.com/OCA/stock-logistics-warehouse/blob/11.0/stock_request/security/ir.model.access.csv#L14

@HviorForgeFlow
Copy link
Member

HviorForgeFlow commented Nov 28, 2019

@bodedra
Ugly form to fix error:

if self.warehouse_id != loc_wh:
    self.location_id = self.warehouse_id.sudo().lot_stock_id.id
if self.warehouse_id.sudo().company_id != self.company_id:
    self.company_id = self.warehouse_id.sudo().company_id

@HviorForgeFlow
Copy link
Member

@etobella can you figure out how to avoid sudo usage here? I think as you only writing/reading fields it does not matters.

@bodedra
Copy link
Member Author

bodedra commented Nov 30, 2019

This PR is 🍏 and ready for your review.

@b-kannan @dreispt @max3903 @gavindav @diverfr @MDodoo @guimarc-br

@bodedra
Copy link
Member Author

bodedra commented Nov 30, 2019

Thank you @hveficent for suggested fix in stock_request_* module.

@rousseldenis rousseldenis added this to the 11.0 milestone Nov 30, 2019
Copy link
Member

@nikul-serpentcs nikul-serpentcs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpicking

Squash Commit's

scrap_reason_code/__manifest__.py Outdated Show resolved Hide resolved
@bodedra bodedra force-pushed the 11.0-scrap_reason_code branch 2 times, most recently from 3794bbb to a50acf1 Compare January 23, 2020 14:47
@bodedra
Copy link
Member Author

bodedra commented Jan 23, 2020

@nikul-serpentcs Comment addressed! Thanks for your review.

Copy link
Member

@nikul-serpentcs nikul-serpentcs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review LGTM 👍

@bodedra
Copy link
Member Author

bodedra commented Mar 27, 2020

@rousseldenis Comment attended!

@bodedra
Copy link
Member Author

bodedra commented Mar 27, 2020

@Chandresh-SerpentCS are you available for a review?

Copy link
Sponsor Contributor

@rousseldenis rousseldenis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review

scrap_reason_code/README.rst Show resolved Hide resolved
scrap_reason_code/models/stock_scrap.py Outdated Show resolved Hide resolved
@bodedra
Copy link
Member Author

bodedra commented Mar 28, 2020

@rousseldenis Comment attended! Would you please review it again.

@bodedra
Copy link
Member Author

bodedra commented Mar 30, 2020

@OCA/logistics-maintainers are we good to merge this?

@bodedra
Copy link
Member Author

bodedra commented Apr 1, 2020

Ping @dreispt

@rousseldenis
Copy link
Sponsor Contributor

@bodedra Do you want to keep the 3 commits or do you plan to squash them ?

@bodedra
Copy link
Member Author

bodedra commented Apr 1, 2020

@rousseldenis Let me squash them into 1.

@rousseldenis
Copy link
Sponsor Contributor

/ocabot merge

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 11.0-ocabot-merge-pr-770-by-rousseldenis-bump-no, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit e831ec3 into OCA:11.0 Apr 1, 2020
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 5ec1ba5. Thanks a lot for contributing to OCA. ❤️

@bodedra bodedra deleted the 11.0-scrap_reason_code branch April 1, 2020 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants