[18.0][MIG] stock_vertical_lift: Migration to 18.0 #2255
Conversation
* Add vertical_lift_shuttle_id field on stock.location, help to find the shuttle for a location * Add StockLocation.fetch_vertical_lift_tray(), that needs to be implemented in addons to send commands to the hardward to fetch a tray, and if existing show a cell (laser pointer, ...) * Add helpers on stock.move.line fetch_vertical_lift_tray_source() and fetch_vertical_lift_tray_dest() that fetch the tray directly from a move line's source or destination location
Namely, the pick/put/inventory operations are now split in different models. Pick and Put share a model and customize their behavior, which is pretty similar. The inventory operation will have a different view and different workflow. This changes will ease a lot the customization of the different workflows and views.
When we refresh the page on the browser when we are using the "screen" view, odoo loses the information that we want the view to be headless, fullscreen, etc. so it's displayed pretty badly. This view is a work-around: its priority is lower, so it will be picked up by default on loading, and a button allows to re-open the screen view with the proper options.
There is no such action as 'ir.actions.do_nothing', it kinda works, until you look into the js console and stares at the errors. There is a nice OCA module that serves this purpose (more or less, because it reloads the window, this is not an issue).
Example of usage in an odoo shell, when a screen is open: >>> self.env['vertical.lift.shuttle'].browse(1)._operation_for_mode().operation_descr = 'foo' >>> self.env['vertical.lift.shuttle'].browse(1)._send_notification_refresh() >>> env.cr.commit() Provided the longpolling is correctly configured with a proxy, the screen should immediately refresh with 'foo' as operation description.
not a list
* The change of destination location was not updated on the screen when the barcode was scanned (it was when the "manual barcode wizard" is used though) * We should be able to pick partially available move lines * prevent to scan a location when no move line is selected or the move line has already been set to done
Instead of going through the onchange machinery. The intended usage of onchange methods is to update something on the screen, without side-effects in the database, then let the user save the form with the proposed changes. Weirdly, the barcode scanner event triggers an onchange on the field `_barcode_scanned`. It doesn't work well with our use case, as the whole form is read-only and we only care about having the barcode events doing side-effects on the backend and displaying back the changes. This particular onchange will then be executed as a normal method, with side-effects. However, contrarily to other actions on the form, the frontend does not reload the view after an onchange, as it relies on the data returned back in the values. As we cannot know which values may have been changed in the different implementations (location destination, state, ...), the onchange returns a read with every field.
The documentation of the state machine is in VerticalLiftOperationBase._transitions.
Compatibility module between stock_vertical_lift and stock_storage_type (in OCA/wms). In the vertical lift's Putaway screen, when a good is scanned for a putaway, the user has to scan the tray type of the corresponding size, so an empty place in a matching tray is found. When we use storage types, we should know what tray is compatible with the storage type. Changes with this module: * The storage types of trays cannot be selected in the locations form, they have to be set in the Tray types. * In the lift put-away screen, when a package has a storage type, the user isn't asked to scan a tray type, instead, the putaway of the Package Storage Type is applied.
When the shuttle screen propose a tray based on a tray type and we are in the 'save' step, where we are supposed to physically putaway the good and save, we should still be able to change the tray type to fetch another tray.
* Rename methods that fetch a tray to prevent confusion * Add methods to release a tray * The Kardex method to fetch a tray has to send "0" in the carrier and carrierNext field * The pick and inventory screens release the tray only when there is no next line, because the release is implicit when we fetch the next line, the put screen releases everytime because the operator may take time to start the next line and we don't know if they are going to scan a next line or not. * Exiting the screen or switching screen between put/pick/put-away has to release the tray as well.
As the template is not used by JS we can pass full objects to it. This way we can use any recordset information directly in the template without having to override the method.
* stock_vertical_lift: make pkg compute more solid Somehow sometimes you can get a move line without product while computing product packaging in inventory. Make it more defensive and skip packaging rendering if no product is there.
The check was means as an optimization: no need to fetch at tray already open. But "fetch_tray" will not only open the tray, it may also move the laser on the exact position. So we should do it for every inventory line.
If we have several goods to put in the same tray, it is inefficient to release (close) the tray between each line if we reopen the same tray. Release the tray only when the last line is reached.
Currently translated at 7.1% (9 of 126 strings) Translation: stock-logistics-warehouse-14.0/stock-logistics-warehouse-14.0-stock_vertical_lift Translate-URL: https://translation.odoo-community.org/projects/stock-logistics-warehouse-14-0/stock-logistics-warehouse-14-0-stock_vertical_lift/it/
If the same user opens multiple shuttles operations on different screens, notifications for a specific shuttle will be displayed on all of them, regardless of the shuttle operation that triggered the notification. This commit allows filtering notifications per screen, so that only screens displaying shuttle operations related to the current notifications will display them. Notifications that are not shuttle-related are not filtered out.
|
/ocabot migration stock_vertical_lift |
| <span | ||
| class="packaging_name" | ||
| itemprop="name" | ||
| t-esc="packaging.name" |
There was a problem hiding this comment.
t-esc directives are deprecated
| # give the unique shuttle for any location in the tree (whether it's a | ||
| # shuttle, a tray or a cell) | ||
| inverse_vertical_lift_shuttle_ids = fields.One2many( | ||
| comodel_name="vertical.lift.shuttle", inverse_name="location_id", readonly=True |
There was a problem hiding this comment.
If possible avoid readonly fields
| tray_type_code = fields.Char(compute="_compute_tray_data", string="Tray Code") | ||
| tray_x = fields.Integer(string="X", compute="_compute_tray_data") | ||
| tray_y = fields.Integer(string="Y", compute="_compute_tray_data") | ||
| tray_matrix = Serialized(string="Cells", compute="_compute_tray_data") |
There was a problem hiding this comment.
IMO we can just use the Json field and drop the base_sparse_field dependency
| class VerticalLiftController(http.Controller): | ||
| @http.route(["/vertical-lift"], type="http", auth="public", csrf=False) | ||
| def vertical_lift(self, answer, secret): | ||
| if secret == self._get_env_secret(): |
There was a problem hiding this comment.
🔴 This seems unsafe.
I know this change isn't introduced by the migration, but we could take the opportunity to improve this
I don't see this VERTICAL_LIFT_SECRET variable documented anywhere, and the default is just an unsecured endpoint
IMO we should check this secret is set, and fail with a message explaining how to set it.
If an unprotected (no secret) option wants to be given, IMO it should be explicit (e.g.: by setting the secret to False, for example)
Additionally, it'd be better to not rely on an environment variable for this secret. Some environments like Odoo.sh don't allow customizing these variables. Instead, we should read this from an ir.config_parameter instead. The config parameter itself can optionally be configured through environment with the usage of https://github.com/OCA/server-env/tree/18.0/server_environment_ir_config_parameter (no direct dependency needed)
There was a problem hiding this comment.
improved to read secret by config_param
| # We reload mainly because otherwise, it would close | ||
| # the popup. This action is provided by the OCA module | ||
| # web_ir_actions_act_view_reload |
There was a problem hiding this comment.
| # We reload mainly because otherwise, it would close | |
| # the popup. This action is provided by the OCA module | |
| # web_ir_actions_act_view_reload |
misleading/outdated comment. In fact soft_reload is a core feature now, that module is deprecated
LucasTran380381
force-pushed
the
18.0-mig-stock_vertical_lift
branch
from
0097e6d to
42a0bbb
Compare
| _logger.error("secret is not set") | ||
| msg = self.env._( | ||
| "Vertical Lift secret not set. " | ||
| "Pls set secret in Inventory/Settings/Vertical Lift" |
There was a problem hiding this comment.
| "Pls set secret in Inventory/Settings/Vertical Lift" | |
| "Please set it in Inventory/Settings/Vertical Lift" |
| .get_param("stock_vertical_lift.secret", None) | ||
| ) | ||
| if not secret: | ||
| _logger.error("secret is not set") |
There was a problem hiding this comment.
Please a little bit more verbose with this message 🙏🏻
Mention also how to configure it.
You can probably use the same message than the exception
There was a problem hiding this comment.
I use the same message than the exception to make it more verbose. Thanks for suggestion
LucasTran380381
force-pushed
the
18.0-mig-stock_vertical_lift
branch
from
42a0bbb to
07350dd
Compare
| "move_line_ids": [], | ||
| "backorder_id": picking.id, | ||
| } | ||
| ) | ||
| new_picking.message_post( |
There was a problem hiding this comment.
Why removing this message?
| @@ -351,7 +343,9 @@ def _send_notification_refresh(self): | |||
| "action": "refresh", | |||
| "params": self._get_user_notification_params(), | |||
| } | |||
| self.env["bus.bus"].sendone(channel, bus_message) | |||
| self.env["bus.listener.mixin"]._bus_send( | |||
There was a problem hiding this comment.
self.env.user._bus_send( ❓
Looks weird to call the function on the mixin
There was a problem hiding this comment.
Hi @grindtildeath, You can take a look bus.py #Line102
This docstring explains a low-level method used to send notification_type and message to a specific target. It recommends using _bus_send() from bus.listener.mixin for better simplicity and security. When using _sendone() directly, it's important to ensure that the target (if it's a string) is not guessable by an attacker to prevent unauthorized access.
| line_model = self.env["stock.quant"] | ||
| record.number_of_ops = line_model.search_count( |
There was a problem hiding this comment.
I would suggest to rename to quant_model if it's not an inventory line anymore.
| float_compare( | ||
| line.theoretical_qty, | ||
| stock_quant.quantity, | ||
| self.quantity_input, | ||
| precision_rounding=line.product_uom_id.rounding, | ||
| precision_rounding=stock_quant.product_uom_id.rounding, | ||
| ) | ||
| == 0 |
There was a problem hiding this comment.
You could use float_is_zero there
| stock_quant = self.quant_id | ||
| if not stock_quant.vertical_lift_done: | ||
| stock_quant.vertical_lift_done = True | ||
| if self.quantity_input != stock_quant.inventory_quantity: |
Kimkhoi3010
force-pushed
the
18.0-mig-stock_vertical_lift
branch
from
07350dd to
ba36402
Compare
Depend
#2257
Put from receipt
Pick from Delivery
Adjust Inventory