-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[DISTANCE] [Tracking] LOW: Refactor UpdateDistanceRequest in Auth/PHP/App to follow 1:1:1 pattern and split it out per field #28358
Comments
Job added to Upwork: https://www.upwork.com/jobs/~010ea3e1f42f2fd6dd |
Triggered auto assignment to @kevinksullivan ( |
Triggered auto assignment to Contributor Plus for review of internal employee PR - @allroundexperts ( |
@allroundexperts There won't be anything for you to do on this for a while since I will be working on the server APIs. Once those are done, I'll have to implement it in the UI, which I will then need help on and I'll get it auto-assigned at that time. @kevinksullivan for the same reason, I won't need you assigned to this for now either. Can you please remove the upwork job? |
Closed the job 👍 |
I'm going to split these into individual issues as I work on them. Each field is going to need an Auth PR, a Web-E PR, and an App PR. First issue is #29993 for date. |
After discussing this with @cead22 in Slack we decided to narrow the scope of this refactoring to PHP only and to leave the Auth command as it is. That should work. It will mean that we need to call Auth to get the full transaction first, then modify the single field that the frontend is updating, and pass the entire transaction to |
This is still moving along at a weekly pace. I'm actually going to move this to monthly since it's the tracking issue and doesn't need updated as frequently. |
Split out all the tasks into their own issues today. The first task has 1 PR merged (should be merged tomorrow), 1 in review, and 1 in draft still. |
This is coming along pretty nicely! |
This is on track to finish soon. Most issues have been closed and just waiting on one last one. |
👋 Is this done? Moving it to #wave-collect polish in the meantime. |
Looks like this is all done! 🎉 |
Problem
The API command
UpdateDistanceRequest
in PHP was created to take the entire transaction object and validate/save it. This lead to the UI re-using the same command for multiple actions: updating amount, updating description, updating category, etc.. This is bad because when editing the description, the API command expects the entire object to be passed and be valid. If there are invalid waypoints, for example, you cannot update the description. This leads to lots of front-end hacks to get around issues related to errors.Solution
Split the command out into field specific commands:
cc @neil-marcellini @Gonals
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: