-
-
Notifications
You must be signed in to change notification settings - Fork 130
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
Feat/barcode v2 #4732
Feat/barcode v2 #4732
Conversation
This changes the behavior of the endpoint if the task is linked to a delivery. Instead of only assigning the task in question, the entire delivery is assigned. This may be less compliant with REST principles, but leaving it as it was without any validation would have allowed clients to assign a delivery to different riders.
This change is related to the previous commit.
c009298
to
3eb16e1
Compare
e2068ae
to
2498e92
Compare
7e294fd
to
9b0ab19
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.
hello it seems good to me
can you check this bug and if it happens when you assign a delivery with a barcode? (drop is before pickup in the list) because I use the same functions as you
#4725
also there may be a UI/UX problem from:
- assign delivery1 with barcode
- assign delivery2 with barcode
then I think the rider will have the following assignments
- delivery1.pickup
- delivery1.dropoff
- delivery2.pickup
- delivery2.dropoff
but you may want to have all the pickups assigned first in the tasklist then the drops to be done afterwards
@@ -76,25 +79,45 @@ public function barcodeAction( | |||
* Possible actions: ask_to_assign, ask_to_unassign | |||
* ask_to_assign: Will prompt the user to self-assign |
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.
shall we have ask_to_start
in the workflow? or maybe assign the task + start the task?
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.
Yeah good idea ! But i'm expecting changes here. I'm pretty sure the auto-assign on scan can fire back on multi points delivery
This PR Includes
assignTo(User)
andunassign()
methods to theDelivery
entity./api/tasks/:id/{un}assign
Endpoints:{un}assign
endpoints now apply to the entire delivery if the task is linked to one. Initially, I considered adding separate methods on theDelivery
entity endpoint, but decided against it to maintain data integrity directly via a single endpoint./api/barcode
Endpoint:To Be Completed
BarcodeController
methods (consider using a Voter)