-
-
Notifications
You must be signed in to change notification settings - Fork 239
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
[16.0][IMP]stock_report_quantity_by_location: Included report generation #345
base: 16.0
Are you sure you want to change the base?
Conversation
anusriNPS
commented
Oct 4, 2024
•
edited
Loading
edited
c45c8f7
to
814074a
Compare
5229804
to
573e62a
Compare
0c9481d
to
4633d3d
Compare
4633d3d
to
b0558ec
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code review: minor fixes
new_context = safe_eval(new_context) | ||
except (TypeError, SyntaxError, NameError, ValueError): | ||
_logger.warning( | ||
_("Failed context evaluation: %(context)s", context=new_context) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: let's maintain the same string translation/formatting structure as the other modules
_("Failed context evaluation: %(context)s", context=new_context) | |
_("Failed context evaluation: %(context)s") % {"context": new_context} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Modified as suggested
action = self.env.ref( | ||
"stock_report_quantity_by_location." | ||
"action_stock_report_quantity_by_location_html" | ||
) | ||
vals = action.read()[0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue: we should use the proper way to open an action
action = self.env.ref( | |
"stock_report_quantity_by_location." | |
"action_stock_report_quantity_by_location_html" | |
) | |
vals = action.read()[0] | |
action = self.env["ir.actions.act_window"]._for_xml_id( | |
"stock_report_quantity_by_location." | |
"action_stock_report_quantity_by_location_html" | |
) |
Following this change, rename the following instances of vals
with action
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Used "ir.actions.actions" to get an action
Included pdf report generation functionality for module stock_report_quantity_by_location
b0558ec
to
1b1bb34
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code review: LGTM