Implement devhub UI to trigger version rollback

[eviljeff]

Fixes: mozilla/addons#15660 + fixes mozilla/addons#15666

Description

Adds a devhub modal form that can trigger a rollback to a previous version, along with a waffle switch which ensures the feature is only enabled when ready.

Context

The messaging will be updated with mozilla/addons#15662 but this PR should be functionally complete. There are some differences to the Figma mockup to match the norms already used in the version list - e.g. we don't show the AMO/Self badges until the developer has uploaded versions in both channels - and because of comments where it was clarified we shouldn't offer a selection of versions for rollback for listed, but a single version.

Release notes and suggested versions are separate issues because they were marked as lower priority requirements in the PRD

Testing

• navigate to a /versions page in devhub for an add-on. The rollback button and form is only shown if the following criteria is met, so check that one of them not being met means the button isn't shown:
  • waffle switch version-rollback enabled in django admin (or with ./manage.py command)
  • there are at least 2 approved versions in at least one of the channels (i.e. you have to have something to rollback to, and a latest version to roll over)
• with the waffle switch enabled and 2 approved versions in either the listed or unlisted channels, see the button is available above the version list
• the button shows the rollback form
• verify the channel selector:
  • if the add-on has only listed versions the channel selector will be hidden in the form (and set to listed)
  • if the add-on has only unlisted versions the channel selector will be hidden in the form (and set to unlisted)
  • if the add-on has mixed channels the channel selector radio buttons will be shown and default to the most channel of the most recent version.
  • clicking on AMO or Self will show/hide the display of the select with single version that will be rolled back in the listed channel, and the select with (potentially multiple) versions in the unlisted channel
• for the new version number, specify a version number that's already been used (from any channel, with any approval or deletion status) and see the form returns an error.
  • (this isn't a new function - it's already used during version upload)
  • repeat for listed and unlisted
  • the redirect back to /versions should have the form already opened
• for listed channel only, specify a lower version than the highest signed listed version, and see the form returns an error
  • (this isn't a new function - it's already used during version upload)
• successfully rollback a version in either channel.

Checklist

• Add #ISSUENUM at the top of your PR to an existing open issue in the mozilla/addons repository.
• Successfully verified the change locally.
• The change is covered by automated tests, or otherwise indicated why doing so is unnecessary/impossible.
• Add before and after screenshots (Only for changes that impact the UI).

[KevinMind]

Merged the CI fix, so feel free to rebase and we can continue with review/verify.

[KevinMind]

Summary

1. We should enable/disable the otions for "amo" and "self" depending if either is actually possible to rollback for. Otherwise you get weird UI states. Similarly, if both are available we should pre-select one

2. several general UX comments

Mostly the feature works but could definitely use some polishing.

Specific notes and verification results

Only 1 (listed) approved version with waffle enabled

1 listed 1 unlisted waffle enabled

2 listed 1 unlisted waffle enabled

Similar trend, if we cannot chose a valid unlisted rollback version, we should simply disable that as an option instead of showing an empty select box.

1 listed 2 unlisted waffle disabled

1 listed 2 unlisted waffle enabled

Bug - the "amo" option should be disabled if only unlisted rollback is possible. and vice versa.

2 listed 2 unlisted (1 listed not approved) waffle enabled

Similarly as above, "amo" should be disabled

Observation

If both "amo" and "self" are available, we should preselect one (maybe listed, or maybe based on the channel of the latest approved uploaded version) so fewer clicks are required to complete the form. It also feels weird having a radio with no selection and another field that depends on its value being not disabled.

2 listed 2 unlisted waffle enabled

Expectations were met.. though I think keeping a select with only one option would be the more intuitive representation of the UI. Making it arbitrary text in a form makes it looks like it's not an input, it is and it's weird that it switches from one type to another when switching the channel type.

Screen.Recording.2025-07-21.at.21.04.39.mov

Observation

I would imagine some developers might need a bit of explanation for the new version field. The explanation is there, but it's not next to the relevant field? Why don't we inline that text to be under the new version field so it is more contextually apparent.

Observation

There is a really jarring flash of content when the modal renders on an error submission. It's also kind of weird that there is an error behind the modal and in front of it. And I cannot really read the one behind.

Screen.Recording.2025-07-21.at.21.07.54.mov

Further, is there any kind of validation to check for on the version input field? Other than not an existing version

Observation

When I successfully submitted the modal, it was a bit jarring that it disappeared and then there was a success notification in the main page. This was out of context for me and it took a second to realize that the notification indicated what I did in the modal was successful. We should probably show the success screen in the modal.

Observation

When I submitted a rollback for unlisted, and then visited the unlisted review page, I got an action "approve multiple versions" but no versions available to select. How am I supposed to approve the new version?

Observation

The state for the modal is odd. I would think if I press cancel we should clear the form, as I cancelled it. but even across refresh the state is synced. I could see syncing it if I refresh in the middle without cancelling, but cancelling should clear it.

Screen.Recording.2025-07-21.at.21.14.51.mov

[eviljeff]

Many many comments so sorting them..

Okay, sure.

We should enable/disable the otions for "amo" and "self" depending if either is actually possible to rollback for. Otherwise you get weird UI states.

It currently just defaults to the one that's possible because I thought otherwise it may be confusing to the developer why it wasn't available? I can change it 🤷

If both "amo" and "self" are available, we should preselect one (maybe listed, or maybe based on the channel of the latest approved uploaded version)

Feels a bit magical and under specified, but I guess...

There is a really jarring flash of content when the modal renders on an error submission. It's also kind of weird that there is an error behind the modal and in front of it. And I cannot really read the one behind.

Okay, I can drop the error message behind.

Question for others

When I successfully submitted the modal, it was a bit jarring that it disappeared and then there was a success notification in the main page. This was out of context for me and it took a second to realize that the notification indicated what I did in the modal was successful. We should probably show the success screen in the modal.

The figma clickable prototype shows:

which I interpreted as a success message on the main page, like you get for other types of actions in devhub. Maybe @Piratesarereal can clarify?

though I think keeping a select with only one option would be the more intuitive representation of the UI.

I did have that at one point during development, but mozilla/addons#15660 (comment) says "This also means that part doesn't need a version-select option, because there can only be one such version" @wagnerand wdyt?

Question for Kevin

It also feels weird having a radio with no selection and another field that depends on its value being not disabled.

Not sure what you mean here.

Replies

so fewer clicks are required to complete the form.

Making the form have as fewer clicks as possible wasn't a project goal. We want the developer to think about what they're doing, I figured.

When I submitted a rollback for unlisted, and then visited the unlisted review page, I got an action "approve multiple versions" but no versions available to select. How am I supposed to approve the new version?

You aren't supposed to approve it - it's auto approved. From https://mozilla-hub.atlassian.net/browse/AOP-538 "allow a developer to roll-back their users to a previously approved version of an extension immediately"

The state for the modal is odd. I would think if I press cancel we should clear the form, as I cancelled it. but even across refresh the state is synced. I could see syncing it if I refresh in the middle without cancelling, but cancelling should clear it.

It wasn't specified either way, does it matter? It's just Firefox remembering form field values.

Further, is there any kind of validation to check for on the version input field? Other than not an existing version

Just the ones specified in the PRD - greater than the current version for listed.

I would imagine some developers might need a bit of explanation for the new version field. The explanation is there, but it's not next to the relevant field? Why don't we inline that text to be under the new version field so it is more contextually apparent.

The messaging will be updated with mozilla/addons#15662

> several general UX comments

That's why there's a waffle switch :) So we don't have to iterate until the UX is perfect in a single PR

[Piratesarereal]

which I interpreted as a success message on the main page, like you get for other types of actions in devhub. Maybe @Piratesarereal can clarify?

Yes, that is a success toast on the main page. The only thing is to use the header strings and body strings as specified in the image.

[wagnerand]

I did have that at one point during development, but mozilla/addons#15660 (comment) says "This also means that part doesn't need a version-select option, because there can only be one such version" @wagnerand wdyt?

If it's easier to keep the select in with only one option and @Piratesarereal is ok with that, sure. Ideally we should pre-select the only available option then, if we aren't doing that already.

[KevinMind]

I did have that at one point during development, but mozilla/addons#15660 (comment) says "This also means that part doesn't need a version-select option, because there can only be one such version" @wagnerand wdyt?

Yes a select with a single option satisfies those requirements and is more indicative of the potential decision that is not currently available due to their only being one option.

It also feels weird having a radio with no selection and another field that depends on its value being not disabled.

Im' referring to the scenario 2 listed 2 unlisted waffle enabled as well as the observation If both "amo" and "self" are available, we should preselect one .

There are 3 fields:

• channel
• version to rollback
• new version

2 depends on 1 and 3 depends on 2. 1 and 3 are immediately visible so I could enter a value, but then select an option for 1 that shows me 2 (may or may not be disabled) and depending on what I select (if there is an option at all might retroactively make my selection for 3 invalid. It's just ... weird.

If we pres elect an option for 1 we avoid that because all fields are always visible and in the correct state on form entry given the selections for other fields.

does that clarify?

[KevinMind]

@eviljeff I'll leave it up to you to decide when it's ready to re-verify. LMK or if I'm waiting on some pending commits. Some of the comments seem to have been accepted, some rejected. So I'm expecting at least some further changes.. Please clarify.

[eviljeff]

@eviljeff I'll leave it up to you to decide when it's ready to re-verify. LMK or if I'm waiting on some pending commits. Some of the comments seem to have been accepted, some rejected. So I'm expecting at least some further changes.. Please clarify.

I've made the changes to preselect one of the channels when both are possible, and to disable the channel selector otherwise. The error message in the versions page header is removed. The success message in the versions page header is retained, based on aarjav's confirmation that's what he intended. The listed version text is now a select, like for unlisted.

Ready for review again.

[KevinMind]

@eviljeff would you rebase and re-request review when all resolved?

[KevinMind]

After submitting the modal, should I expect to see the new "rollback" version. in the version history as awaiting review or anything? Seems odd that there is no paper trail in devhub.

While submitting for listed, I got a form focus error that prevented submission silently 😨

Screen.Recording.2025-07-29.at.21.12.13.mov

Generally, seems to work.. but I'm a bit worried about the form input error though.

[eviljeff]

After submitting the modal, should I expect to see the new "rollback" version. in the version history as awaiting review or anything? Seems odd that there is no paper trail in devhub.

You won't expect to see anything in the request/response cycle of the request that launches an async task, no - duplicate_addon_version_for_rollback.delay(...).

Also, is this behaviour only apparent now? There's nothing that I (purposefully) changed in the core functionality in the past 2 weeks.

[eviljeff]

While submitting for listed, I got a form focus error that prevented submission silently

I can confirm. Weird.

[eviljeff]

While submitting for listed, I got a form focus error that prevented submission silently

Fixed - the <select> was being rendered with required so it was blocking the form submission. Rather than trying to work around that in javascript I added required=False because we already have (custom) form validation that requires a version be selected if nessecary.

[KevinMind]

Is there a logic condition when it would be required or not required? I assume depending on the selection of the listed channel right?

I guess that's fine.. to bad it would be nice to use html validation but alas we have no "react"ivity in our hub of devs.

[KevinMind]

I mean I totally could and did expect to see something. It's odd UX to create a version, see the success indicator and then just have no visibility on it until some arbitrary point in the future. We don't even communicate that there will/might be something later. But happy to let that be something we don't fix or fix later. CC @Piratesarereal should look at this at some point.

After submitting the modal, should I expect to see the new "rollback" version. in the version history as awaiting review or anything? Seems odd that there is no paper trail in devhub.

You won't expect to see anything in the request/response cycle of the request that launches an async task, no - duplicate_addon_version_for_rollback.delay(...).

Also, is this behaviour only apparent now? There's nothing that I (purposefully) changed in the core functionality in the past 2 weeks.

[eviljeff]

Is there a logic condition when it would be required or not required? I assume depending on the selection of the listed channel right?

The unlisted version is required if unlisted is chosen in the channel select (and similarly with listed, but there's only one selection so it's always there). But like I said, we have form validation that we have an appropriate version for the selected channel.

[eviljeff]

I mean I totally could and did expect to see something. It's odd UX to create a version, see the success indicator and then just have no visibility on it until some arbitrary point in the future. We don't even communicate that there will/might be something later. But happy to let that be something we don't fix or fix later. CC @Piratesarereal should look at this at some point.

The flow is exactly as the Figma prototype in the issue.

The engineering implementation of the task that does the actual rollback was designed around how the prototype works - what you're suggesting is not possible without fundamentally redesigning how the feature works.

[KevinMind]

The form issue seems to be resolved. In verifying this patch I rolled back about 5-10 versions. I see nothing anywhere documenting which versions I've currently rolled back, their pending status, etc.

I think we should consider if that is a good user flow from the developer's perspective for future iterations, even if it is a fundamental change.

[KevinMind]

For the future, since we discussed timing on processing this PR. It is ~1k lines of relatively complex user facing features. That is inherently difficult and time consuming to review/verify. Several bugs were found, identified and squashed, all taking time. Each time I have to re-run the full set of steps to make sure one fix didn't open a new bug.

You might have considered breaking the PR into smaller PRs that would add chunks of the larger feature. Especially since this feature is behind a flag it is trivial to ship parts at almost any arbitrary slice size. These could be more quickly verified/reviewed and could have led to faster incremental progress and perhaps a faster overall time to approved. Do with that feedback what you will but just a thought for the future.

[eviljeff]

For the future, since we discussed timing on processing this PR. It is ~1k lines of relatively complex user facing features. That is inherently difficult and time consuming to review/verify. Several bugs were found, identified and squashed, all taking time. Each time I have to re-run the full set of steps to make sure one fix didn't open a new bug.

You might have considered breaking the PR into smaller PRs that would add chunks of the larger feature. Especially since this feature is behind a flag it is trivial to ship parts at almost any arbitrary slice size. These could be more quickly verified/reviewed and could have led to faster incremental progress and perhaps a faster overall time to approved. Do with that feedback what you will but just a thought for the future.

Yes, absolutely agree - there was more non-form code change in this than I intended, and potentially even the form could have been split into parts. (The same changes in multiple patches would have likely been more lines of code in total though. For example, I only realised when I was developing the form validation logic that my what I wrote for rollbackable_versions_qs originally didn't work - I was slicing the qs to skip the most recent version; but ModelChoiceField works by filtering the qs, and that's not allowed after a slice, so I had to rewrite it significantly)