add 'send per post' option to the appreciation dialog (draft, needs a BE status field) - #924
Open
ivannissimrch wants to merge 2 commits into
Open
add 'send per post' option to the appreciation dialog (draft, needs a BE status field)#924ivannissimrch wants to merge 2 commits into
ivannissimrch wants to merge 2 commits into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Draft, not for merging. The frontend side is done, but it needs something from the backend, and I'm not sure what the right shape is, so I'm opening this to show what I have and ask.
The problem: an appreciation row only has
dateDueanddateDelivery. There is no status, so the frontend figures out what to show by checking which date is set (Appreciation.tsx,entry.dateDelivery ? "received" : "pending"). With two options, that works. With three it breaks. A posted item and a pending item look identical in the database; both have a due date and no delivery date, so when you reopen the dialog, there is nothing to tell them apart and it shows pending.What I tried: I added a
statusfield on the appreciation on a localbebranch, to see if it worked end to end.That was the field on ApiAppreciation in the SDK, a column on the appreciation table, the same property in sdk-types.json so request validation lets it through, and one line in dtoAppreciation. No route logic changed, GET /volunteer/:id/appreciation, POST /volunteer/:id/appreciation and PATCH /appreciation/:id all pick it up as they are. It worked, and the frontend in this PR is written against it. Until the field is in the published SDK, types.ts widens the type locally, so this does not run against dev or prod.
@arturasmckwcz here is what I did and why. Is a status field the right approach, or would you do it differently?
Can you help me get it built on the backend?
One thing I am unsure about either way: the sent date currently goes into
dateDue, because that is the only column that can hold it, which makes the column name a bit off.Related Issues
Closes #640
Changes
Appreciation/types.tswithDeliveryStatusandAppreciationWithStatus, matching the siblingtypes.tsfiles inActivityLog,OpportunityDetailsand VolunteerAgents`statusdirectly instead of an ordered chain of date checksgetStatusLabelbecomes aRecord<DeliveryStatus, ...>lookup, so a fourth status will not compile until it is handledScreenshots / Demos
Checklist