[16.0][IMP] base_pos_self_service_weighing#1530
Conversation
… reverse proxy conficuation to manage CORS
missing XML template
…usage and developing of the module
…ith string concatenation to fix odoo bundling of js files
…roxy configuration
+ pre-commit run
Signed-off-by: Carmen Bianca Bakker <carmen@coopiteasy.be>
Signed-off-by: Carmen Bianca Bakker <carmen@coopiteasy.be>
Signed-off-by: Carmen Bianca Bakker <carmen@coopiteasy.be>
Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
* refactor and improve base_pos_self_service_weighing and pos_self_service_weighing_tare. * complete migration of pos_self_service_base to 16.0.
* migrate pos_self_service_print_zpl to 16.0. * rename module to pos_self_service_weighing_print_zpl. * improve label layout.
add new module pos_self_service_weighing_product_tare.
OCA-git-bot
added
series:16.0
mod:pos_self_service_weighing_tare
huguesdk
left a comment
huguesdk
left a comment
There was a problem hiding this comment.
note: the commit message mentions pos_ss_base, but the module name is base_pos_self_service_weighing. so base_pos_ssw would be more appropriate.
although in the oca guidelines, the example commit messages mention the module name in the subject (summary), it also says “Write a short commit summary without prefixing it. It should not be longer than 50 characters: This is a commit message”; the name of the module can thus be in the commit message itself (not necessarily in the summary). i don’t like the habit of putting the module name in the summary: with the tag prefix we already lose 6 characters, adding the module name leaves really little space to write a meaningful summary fitting in 50 characters. (this is just a personal preference, it seems to go against the most common practice.)
| for record in self: | ||
| record.is_close_session_button_visible = ( | ||
| record.is_self_service_weighing_station | ||
| and record.current_session_state in ("opened", "opening_control") |
There was a problem hiding this comment.
why not simply this?
| and record.current_session_state in ("opened", "opening_control") | |
| and record.current_session_state != "closed" |
There was a problem hiding this comment.
Because there is already a close button in point_of_sale when state == 'closing_control'
There was a problem hiding this comment.
ok, thanks! then maybe using the same text (just “Close”) on the button is more consistent, no?
| self.ensure_one() | ||
| return self.current_session_id.action_pos_session_closing_control() | ||
|
|
||
| @api.depends("is_self_service_weighing_station", "current_session_state") |
There was a problem hiding this comment.
current_session_state is a computed field, and (afaik), odoo doesn’t chain dependencies, so it won’t work. it should be replaced by the dependencies of the compute function of that field: "session_ids", "session_ids.state" (actually, "session_ids" is useless as a "session_ids.state" implies "session_ids" automatically.
|
|
||
| is_close_session_button_visible = fields.Boolean( | ||
| compute="_compute_is_close_session_button_visible", | ||
| store=True, |
There was a problem hiding this comment.
why store=True? this should be avoided if not needed. if not stored, the field will only be computed when displaying the view, while if stored, it will be recomputed each time one of the dependencies changes, even if not used. moreover, this is really more of a ui technical field, so storing this in the database doesn’t make much sense.
There was a problem hiding this comment.
also, since this is only for self-service weighing sessions, it should also have a less generic name. something like is_self_service_weighing_close_button_visible, or maybe a shorter version, like is_ssw_close_button_visible.
| string="Is a Self-Service Weighing Station", | ||
| help="Use this PoS as a self-service weighing station", | ||
| ) | ||
|
|
There was a problem hiding this comment.
no blank lines between field definitions, please.
| <xpath expr="//button[@name='open_existing_session_cb']" position="after"> | ||
| <field name="is_close_session_button_visible" invisible="1" /> | ||
| <button | ||
| t-if="record.is_close_session_button_visible.raw_value" |
| store=True, | ||
| ) | ||
|
|
||
| def close_session(self): |
There was a problem hiding this comment.
this method should have a less generic name, something like close_self_service_weighing_session(), as it should not be used with regular pos sessions.
There was a problem hiding this comment.
a comment is needed explaining the assumptions done here (and the logic). it should explain that normal sessions are closed with close_session_from_ui(), and that this function is a simplified version, assuming that there are no pos orders and thus no account moves to post.
|
|
||
| def close_session(self): | ||
| self.ensure_one() | ||
| return self.current_session_id.action_pos_session_closing_control() |
There was a problem hiding this comment.
as close_session_from_ui() does, after calling this, it should also do this:
self.message_post(body='Point of Sale Session ended')
without this, each session has only a message about its opening, but not about its closing.
7 participants
Add a button in the PoS dashboard to close the session of a self weighing session.
Build upon #1397