feat(labels): add view label action for individual entities - #283
Conversation
…ompany
The entity label action reached for a new internal `labels/{id}` route wired
across into the public `Api\v1\LabelController`. The internal namespace already
exposes `orders/label/{id}` via `Internal\v1\OrderController@label`, which
already resolves `type=entity` through `findEntityLabelSubject()`, so no new
backend route is needed. `$type` also defaults to `strtok($publicId, '_')`, so
an `entity_*` public id resolves on its own and the query param can go.
- drop the added internal `labels` route group
- call `orders/label/{public_id}?format=base64`, matching `viewWaypointLabel`
- reuse `modals/order-label` instead of cloning it, as the waypoint action does,
with an `@options.subject` fallback so the object alt still resolves
- fix the `Failed to load entity label.s` typo and add the two new keys to the
six other locales that already carry the waypoint equivalents
Also scopes label subject resolution to the session company. The lookups matched
on identifier alone, so any authenticated user could render a label for any
order, waypoint or entity in another organization by supplying its public id.
The identifier match is grouped in a closure — appending the company constraint
to the existing chain would read as `public_id = ? OR (uuid = ? AND company_uuid
= ?)` and still leak. Resolution fails closed when there is no company session.
Applied to both the internal and public API paths, with regression coverage for
the foreign-company, precedence and no-session cases.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Thanks for this @janni1288 — and for finding the template bug in #280 first, that was the right place to start. The frontend approach is solid and the action/modal shape matches how we do order and waypoint labels. Two things needed changing before this goes in. Since maintainer edits are enabled I've pushed a commit to your branch rather than send you round again — please look it over. 1. The backend already exposes thisThe internal namespace already has It also pointed at The frontend now calls the same endpoint the waypoint action does: const { data: pdfStream } = await this.fetch.get(`orders/label/${entity.public_id}?format=base64`);
2. Cross-tenant leak in the label lookupsNot something you introduced — reviewing this surfaced it. The lookups matched on identifier alone: Entity::where('public_id', $publicId)->orWhere('uuid', $publicId)->withoutGlobalScopes()->first();We have a Worth knowing if you hit this pattern elsewhere: appending the company constraint to the existing chain reads as Smaller things folded in
On the two open questionsPlacement — sorry for the slow reply on the issue. The entity card next to edit/delete is the right spot; keep it as you have it. "Works consistently across multiple entity types" — you can drop that one. Backend suite and lint pass locally. Once you've looked over the pushed commit I'm happy to merge. |
What
Adds a "View Label" action for individual entities, using the existing
labels/{id}?type=entitybackend endpoint (fixed in #280).Changes
entity-actions.js: newviewLabel(entity)action, mirrors the existingorder-actions.jspatternentity/card.hbs: new "View Label" button next to edit/deletemodals/entity-labelcomponent (template + app re-export), based onmodals/order-labelroutes.php: registered the existing genericlabels/{id}route also within the internal namespace - it was previously only reachable externally, so the console couldn't call itTesting
Note on UI placement
I asked in the issue thread about preferred placement for the new action but haven't heard back yet - went with adding it next to the existing edit/delete buttons on the entity card for now. Happy to move it if you'd prefer a different location (e.g. a dedicated action menu).
Not covered
The acceptance criteria also mention "works consistently across multiple entity types" - I've only tested with a generic entity so far. Happy to verify with other entity types if there are known edge cases I should check, or if this should stay open until that's confirmed.
Part of #182 (backend fix in #280 already merged)