Make reviewer pending rejection input a datetime widget and allow changing it through an action

[diox]

Fixes mozilla/addons#14920

Description

This allows reviewers to change the pending rejection date of versions pending rejection through a widget where they can select the date and time.

Because the actions menu is already pretty large, the clear pending rejection date action is reused, with an option to change or clear ; to tie everything together and make the UI more consistent, the rejection action use the same widgets, making the date choice absolute rather than relative.

Screenshots

New reject multiple versions UI

Modified Clear pending rejection action (now Change pending rejection) UI

Testing

See tests. Non-exhaustive list of scenarios, after submitting an add-on and going to its review page:

• Use the reject multiple versions action with immediate effect
• Use the reject multiple versions action with a delay, and then...
  • Use the change pending rejection action to clear the delay
  • Use the change pending rejection action to change the delay to a date in the future

Changing the delay (not clearing it) should send an email to the developer following copy that can be found as a comment in the issue.

Changing a delay should require the new date to be at least one extra day in the future compared to now. Changing the delay of multiple versions at once is possible, but the existing pending rejection date should all be the same.

[diox]

Drive-by cleanup to reduce confusion - This API was already obsolete since 9f1e462

[KevinMind]

Initial thoughts and feedback/ I'll try to setup and verify tomorrow.

[KevinMind]

@diox Some feedback after running through the steps

It looks like the change pending rejection is available as an action even if I don't have any pending rejections.. not sure if that is pre-existing but seems odd

<img width="1458" alt="Screenshot 2025-01-31 at 14 04 28" src="https://github.com/user-attachments/assets/24ccea36-0

When I schedule a rejection.. shouldn't I see when the pending rejection is for in the log?

fc8-4387-91b3-5ea4e1f815b3" />

It is odd that I see an hr/mm at all, and it is actually html required. Can we remove that to only select a date and not time?

Maybe this is to "react"-ive for what we can do with rev tools but I would expect if I modify the date field, I would exect on blur that we should change the radio to change date and not clear date. It would be super easy to think you are changing the date and actually you clear it. I actually did that before noticing the issue.

Screen.Recording.2025-01-31.at.13.22.47.mov

[diox]

It looks like the change pending rejection is available as an action even if I don't have any pending rejections.. not sure if that is pre-existing but seems odd

That is pre-existing yeah, I didn't change that. We could, I'm not sure it's worth the extra database query (we're trying to minimize them on this page to keep it as fast as possible - every milliseconds counts given how many different things it's loading)

When I schedule a rejection.. shouldn't I see when the pending rejection is for in the log?

You mean the date ? We don't show it in the log because it's not part of the format string, but we do show the current scheduled rejection date in the row of the version itself in the history if the pending rejection is still active.

It is odd that I see an hr/mm at all, and it is actually html required. Can we remove that to only select a date and not time?

Maybe. I played with that a little bit, showing the time was a compromise: I was trying to avoid having all scheduled rejections end up at the same time, and not accidentally reduce the delay by a few hours for some developers depending on when the rejection was posted. My original idea was to only allow reviewers to select the date and automatically set the time to the hour/minute of the submission, and in fact that's shown in my screenshots in the description, but then I changed my mind for this implementation to keep things simpler. I don't feel strongly one way or the other though, but I figured, since we do have a full datetime internally, why not show it.

Maybe this is to "react"-ive for what we can do with rev tools but I would expect if I modify the date field, I would exect on blur that we should change the radio to change date and not clear date. It would be super easy to think you are changing the date and actually you clear it. I actually did that before noticing the issue.

Also something I changed my mind on as I was working on this. My first implementation was more JavaScript-heavy and I went for something a bit simpler to maintain given the current codebase. But I see your point, maybe there is a middle-ground we can reach here. I'll see what I can do.

[diox]

Maybe this is to "react"-ive for what we can do with rev tools but I would expect if I modify the date field, I would exect on blur that we should change the radio to change date and not clear date. It would be super easy to think you are changing the date and actually you clear it. I actually did that before noticing the issue.

Also something I changed my mind on as I was working on this. My first implementation was more JavaScript-heavy and I went for something a bit simpler to maintain given the current codebase. But I see your point, maybe there is a middle-ground we can reach here. I'll see what I can do.

f1f0ac5 does this

[KevinMind]

Verified 🚢