request a reason from the user when suggesting dupes - #2790
Conversation
059be22 to
1e1a84e
Compare
| # reason. The initial GET request (used to check the other person | ||
| # ID before the reason has been entered on the review page) opts | ||
| # out of this by passing require_reason=False. | ||
| self.fields["suggestion_reason"].required = require_reason |
There was a problem hiding this comment.
I see why you've done this, but would it be better to just add a data migration to existing suggestions to add a 'no reason given' stock message, and then we don't need to complicate the rest of the code with this case?
There was a problem hiding this comment.
I can do that, and that would allow us to set blank=False on the model, but I don't think it would change much on the form. The reason this is a bit messy isn't really about the historic data. It is about the inline form on the person page.
Basically I wanted to keep this form
as a single input and just put the big "reason" box on the confirmation screen.
There are two reasons I wanted to do this:
- I didn't want to squeeze a multi-line textbox into that sidebar - it is too much clutter
- That first form interaction passes you to the next page using a GET request.
?person=123is fine in a URL, but if you've written a long justification for why these two people are the same it all ends up in a query param in the URL, which feels messy (and possibly limits the length of response you can provide). I wanted to only show the reason box at the point where your next interaction is a POST.
In order to do that we have to disable the validation of suggestion_reason when we validate the initial GET request.
1e1a84e to
15a7046
Compare
|
Interesting. Merging #2782 changes the base branch of this PR to master and automatically triggers a rebase. |
2 participants
Refs https://app.asana.com/1/1204880536137786/project/1204880927741389/task/1213280348486701?focus=true
In this PR, we give the user a textbox at the point they suggest a duplicate so they can give reasons or supporting evidence which is then surfaced when we review.
I've based this off #2782 which also makes changes to the duplicates flow.
I've also made this PR a stack: https://docs.github.com/en/pull-requests/how-tos/stacked-pull-requests although in this case we might want to merge #2782 independently of this one. I've stacked them more for "avoid doing extra work solving merge conflict" reasons rather than "split one big feature down into easier to review chunks" reasons.