feat(spp_drims): Dispatch page and waybill review — locked lines, one-page waybill - #433
feat(spp_drims): Dispatch page and waybill review — locked lines, one-page waybill#433emjay0921 wants to merge 9 commits into
Conversation
Rendered the waybill and read the result rather than the template. It came out two pages, with every column stacked and the signature row pushed onto page 2; the TO box empty; "Total Items" reading 1 for a 200-unit consignment; the picking's internal note printed; the quantity column showing demand rather than what was picked; and no barcode. Lay it out with tables instead of Bootstrap's grid. Reports are rendered by wkhtmltopdf 0.12.6, a WebKit build with no flexbox, so row/col-* and card collapse and each column becomes a row. A test fails if any of those classes reappear, because the damage is invisible in the browser preview and only shows in the PDF. Fill the TO box from location_dest_id. partner_id is blank on a dispatch — relief goods go to a location, not a customer — which is why the box was empty. Print an address only when a street is recorded, since the contact widget on a bare partner renders its name wrapped in dashes. Show Shipped alongside Demand. A waybill accompanies the goods, so it has to state what is actually on the vehicle; demand stays for reconciliation at the receiving end. Drop the "Total Items" footer, which counted move lines, and the note, which is internal. Always print the Vehicle and Driver rows so there is somewhere to write, and print whichever of Request or Donation matches the transaction type. Add the transaction type, distribution area, estimated beneficiaries and distribution type. The barcode needed two fixes and neither was in the template. reportlab 4.1 defaults renderPMBackend to rlPyCairo, which the image does not install, so /report/barcode/ answered HTTP 500 for every URL form — every barcode and QR code in every Odoo report, not just this one. Odoo's own requirements only pin a backend for win32, assuming the Debian package covers Linux. Adding rlPyCairo fixes the route. The image still did not appear, because an <img> with a relative URL needs wkhtmltopdf to fetch it from inside the rendering process, where web.base.url points at an external host and port that cannot be reached; embedding it as a data URI removes that dependency, verified by rendering with web.base.url deliberately pointed at a dead port. A failure to build the barcode is logged and skipped rather than raised, so it cannot stop a waybill printing. Also expose stock.move's drims_request_line_id and drims_donation_line_id, which appeared in no view, so it is possible to see which request or donation line a move fulfils. Read-only, since reassigning one by hand would misattribute dispatched and delivered quantities. Kept in its own file: the picking form's Operations list is defined inline in stock.view_picking_form and cannot be extended from a separate view. Note the ticket lists a third linkage field, drims_allocation_id, which does not exist on the model. OP#1151
The DRIMS tab sat last, behind Additional Info and Note, although it is where a dispatch is actually worked. It now sits second, right after Operations, and is labelled "Dispatch & Delivery" rather than "DRIMS". Replace "Delivery Address" with the destination location on a dispatch. partner_id is left blank because relief goods go to a location rather than a customer, so the field only invited someone to fill in something meaningless — and its emptiness is why the waybill's TO box printed blank (OP#1151). Core hides location_dest_id for outgoing pickings and keeps an invisible="1" copy behind stock.group_stock_multi_locations, which is why a dispatch showed no destination at all; rather than un-hide core's copy, which would stay invisible wherever that group is off, add a read-only one scoped to dispatches. It is populated by spp.drims.request.action_create_dispatch from the request's destination warehouse. Both changes are scoped to request dispatches, so donation receipts, transfers, returns and plain stock pickings are untouched — asserted per picking kind by evaluating the view's own invisible expressions. The ticket's fourth item, removing the ability to delete a line, is already implemented for request dispatches in OP#1057; repeating it here would mean two xpaths setting the same attribute on the same field. OP#1150
…waybill-and-dispatch-page # Conflicts: # spp_drims/views/stock_picking_views.xml
A request dispatch is generated by action_create_dispatch, which sets the operation type, the source location, the source document, the products and every field under DRIMS Information. None of them is the dispatcher's to change, but core only locks the header fields once the picking is done or cancelled, so they stayed editable for a dispatch's whole working life. Lock them for request dispatches only. Ordinary transfers, donation receipts and returns keep core's behaviour. Quantity is deliberately left editable: entering less than Demand is how a partial dispatch and its backorder are produced (OP#1087). Both header xpaths need qualifying. Core declares picking_type_id twice — once in the header and once column_invisible in the moves list — and location_id three times: a copy hidden behind !stock.group_stock_multi_locations, the visible Source Location behind that same group, and one in the list. position="attributes" takes the first match, which for location_id is the invisible copy, so an unqualified xpath would have locked a field nobody can see and looked like nothing happened. A test asserts the lock lands on the visible field for that reason. Also converts a round-1 xpath from @Class to hasclass(), which the UI hook requires, and moves the test fixtures onto OP#1079's per-warehouse allocation model.
…waybill-and-dispatch-page
…ad-only QA asked whether Source Location had become clickable. It had, and round 2 is what did it: making a many2one read-only turns it into an internal link to the record. Core sets no_open on picking_type_id already, which is why Operation Type is inert - it does not on location_id. no_open cannot be made conditional, because options is parsed as a static dict and cannot reference drims_type. So core's copy is hidden on a dispatch and an inert one shown in its place, the same shape as OP#1158's hazard category. The replacement repeats the multi-locations group, or a single-location installation would start seeing a Source Location field it is not meant to have. The view tests now ask which copy a given kind of picking actually renders, rather than assuming one node per field.
view_stock_move_operations_drims fails the XML ID hook - "operations" is not a view type - so it would block this branch's PR. Renamed to view_move_form_operations_drims, matching its siblings in the same file (view_move_form_drims, view_move_tree_drims). Introduced by the waybill commit on this branch; never on 19.0, and referenced nowhere else, so the rename is contained.
…waybill-and-dispatch-page
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 19.0 #433 +/- ##
=======================================
Coverage 72.24% 72.25%
=======================================
Files 419 419
Lines 29813 29829 +16
=======================================
+ Hits 21539 21553 +14
- Misses 8274 8276 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
gonzalesedwin1123
left a comment
gonzalesedwin1123
left a comment
There was a problem hiding this comment.
Strongest view-layer work of the batch — every risky claim in the PR body verified out against the Odoo 19 core source:
- Every
position="attributes"restatement is exact before the dispatch clause is added:picking_type_id(state in ('done', 'cancel')),origin(state in ['cancel', 'done']),product_id((state != 'draft' and not additional) or move_lines_count > 0), andlocation_id'sinvisible(picking_type_code == 'incoming'). Since attribute replacement is wholesale, a misquote would have silently loosened core behaviour — none did. - The xpath first-match traps are real: core's first
location_idis indeed the invisiblegroups="!stock.group_stock_multi_locations"copy, so the qualified selectors are load-bearing, not pedantry. The inert replacement copy correctly repeats the multi-locations group. - The barcode fix is the right architecture — a data URI removes wkhtmltopdf's dependency on fetching
web.base.urlfrom inside the renderer, which genuinely cannot work in a containerised deployment. The broad-except-warn-return-False in_get_waybill_barcode_data_uriis justified (a barcode must never stop a waybill) and both the helper and the degraded path are tested. - The
rlPyCairoclaim is CI-proven: the test image builds from this checkout's Dockerfile, and the two barcode tests require the dependency and pass. - The 24 tests asserting rendered output (waybill) and per-picking-kind view expressions (dispatch page) prove transfers, receipts and returns keep core behaviour.
Also a pleasing composition: this PR's UI product-lock + #391's create/delete options + #391's server-side validation guard form one coherent layered lock on the dispatch line-up, with Quantity editable throughout for the backorder flow.
1. Blocker — version bump
Per the convention Edwin ruled on (see #391): bump + HISTORY in the PR, number assigned at merge-time rebase. Here it's functionally meaningful, not just signalling: the new stock_move_views.xml and the rewritten waybill template are data, so a deploy without -u spp_drims keeps the old form and the old waybill entirely. Please also have the HISTORY entry mention the required image rebuild for rlPyCairo — that's a deploy-runbook fact, not just a local-testing one.
2. Merge-order note
Your land-this-first proposal makes sense and has been passed to Edwin with one caveat: #393 edits inside the same DRIMS-tab block this PR rewrites and renames, so whichever of #433/#393 lands second has a real rebase rather than a mechanical one. #390/#391 are compatible-but-adjacent as you say.
With the bump added, this is an approve.
Version bump with its changelog entry. Functionally required here rather than signalling: the new stock_move_views.xml and the rewritten waybill template are data, so a deployment that skips the upgrade keeps the old dispatch form and the old waybill in full. The entry also records the deployment fact that the barcode needs rlPyCairo, added to docker/requirements.txt in this branch: upgrading the module is not enough, the image has to be rebuilt, or any report carrying a barcode answers HTTP 500. The waybill still prints without it, minus the barcode.
|
Thanks — bump added, and the deploy note with it. Pushed as 1. Version bump — You are right that this one is functional rather than signalling: The
2. Merge order — noted, and agreed on the #393 caveat: it edits inside the same DRIMS-tab block this branch rewrites and renames, so second-in is a real rebase there. Both are mine, so whichever order you set, I will take the rebase rather than leaving it to the merger. Worth adding that #393 has since had its own review addressed (
|
gonzalesedwin1123
left a comment
gonzalesedwin1123
left a comment
There was a problem hiding this comment.
Approved — the one blocking item is done, and done well.
Verified on 5be1c20a: bump to 19.0.3.0.4 with the HISTORY entry, README/HTML regenerated, and the deployment note is exactly what was asked for — it says plainly that upgrading the module is not enough, the image has to be rebuilt for rlPyCairo, and it correctly records that the waybill still prints without the barcode rather than failing. That's the runbook fact captured where upgraders will see it. Full CI re-ran green after the push.
One mechanical note for merge time, not a change request: #416 (same module) is now approved carrying 19.0.3.1.0 with a migration directory named for it. If #416 lands first, 19.0.3.0.4 becomes a version decrease and won't trigger an upgrade — so per the merge-time-assignment rule, whichever of the two lands second renumbers above the other (and #416's migration folder would move with its number if that's the one renumbered). Your land-#433-first proposal avoids touching #416's migration path, which is another point in its favour; Edwin has both inputs for the queue decision.
Everything else stands from the first review: the core-rule restatements are exact, the xpath disambiguation is load-bearing and correct, the barcode architecture is right, and the 24 rendered-output/view-expression tests hold the scope to dispatches. Merge stays with Edwin per the usual flow.
2 participants
Why is this change needed?
Two children of the Stock Allocation & Dispatch review (OP#1077), both returned from QA as passing:
How was the change implemented?
rlPyCairowas added todocker/requirements.txt: every barcode and QR in every Odoo report goes throughrenderPM, reportlab 4.1 defaults that backend torlPyCairo, and without it/report/barcode/returns 500. Odoo's own requirements only pin a backend for win32, assuming Debian'spython3-renderpm— this image does not install it.spp.drims.request.action_create_dispatchactually populates;partner_idstays blank, and that emptiness is why the waybill's TO box printed blank.no_opencannot be conditional, so core's copy is hidden on a dispatch and an inert copy shown in its place — the same shape as OP#1158's hazard category.picking_type_idtwice andlocation_idthree times, andposition="attributes"takes the first match — which forlocation_idis the copy nobody can see, so an unqualified xpath would have looked like nothing happened.New unit tests
spp_drims/tests/test_waybill_report.py— 13 tests, asserting against the rendered PDF/HTML rather than the template.spp_drims/tests/test_dispatch_page.py— 11 tests, evaluating the view's owninvisibleexpressions per kind of picking so a change scoped to dispatches is proven not to touch transfers, receipts or returns.Unit tests executed by the author
Full
spp_drimssuite on this branch, against a fresh database: 287 tests, 0 failed, 0 errors.Worth knowing for anyone testing locally: two of the barcode tests need the
rlPyCairothis PR adds, so a container image built before this branch fails them withModuleNotFoundError: No module named 'rlPyCairo', andpip install rlPyCairoinside the runtime image cannot fix it — pycairo needs the dev headers that only the builder stage has. Rebuild the image (docker compose build openspp-dev) and they pass. CI builds the test image fromdocker/Dockerfileat this checkout, so it picks the dependency up on its own.How to test manually
./spp start, then open a DRIMS request and allocate it.Related links