lando-ui: merge remaining LandoUI code (Bug 1944562) #228
Conversation
…ve to immutable tags for deployment (#192)
…9) (#194) Create a new modal div that contains the uplift request form. Add a new button for requesting uplifts next to the "Preview Landing" button. Remove the code that allows pressing the "Preview Landing" button even when the stack is not landable since we no longer have the uplift button behind that modal.
…lable (Bug 1888188) (#198) As part of the checks refactor we will eventually support displaying multiple blockers in the front end. Add a shim so Lando-UI can handle output from before and after the refactor PR in LandoAPI is landed and deployed.
Implement a Treestatus UI in Lando. All endpoints in the new interface are namespaced under `/treestatus`. The main components are as follows. The main Treestatus page displays all trees and their current statuses. When authenticated, each tree can be selected for updating via a checkbox, or a "select all trees" button is available for CI-wide tree closures. The tree selections are forwarded to the update form when "Update trees" is selected. The "update trees" view implements a form for updating the status of trees. Trees can be set to open, closed and "approval required". When the status is being set to a non-open state, a reason and reason category must be specified. The form presents a "remember this change" option which can be used to easily revert the change at a later time. For example if all trees are closed, the trees can be mass re-opened easily. The individual log view of a tree presents the most recent status updates for each individual tree. When authenticated, users can edit the tree status reason or category in case of a typo or error during submission by clicking the "edit" button, which toggles the display to a form for updating. The recent changes view is a header that appears for authenticated users on all pages. It presents all status updates that have been "remembered" during update. Users can edit the reason and reason category using an "edit" button that toggles the display to a form. The "restore" button can be used to revert the status change. The "discard" button can be used to throw away the status change if it is no longer necessary to remember the change. The Treestatus UI uses message flashing and the existing Lando-UI notifications component to present errors and action verification to the user.
…r key (Bug 1888188) (#199) In the original revision I added a `blocked_reason` variable to avoid repeated code, however I forgot to update the template to actually use the variable.
…calls (Bug 1896642) (#203) In other parts of Lando, before making an API call that uses the `require_auth0` kwarg, we do an explicit check that the user is authenticated and handle unauthenticated requests appropriately. This was overlooked in implementing the Treestatus UI and is resulting in errors in Sentry since we instead only discover the missing credentials when making the request, resulting in an unhandled exception. Add an explicit check to each handler which makes an `auth0_required` API call.
…202) When calling out to the tree logs endpoint, we do not properly handle non-2XX reponse codes within LandoUI, as those throw a `LandoAPIError` which must be handled. Add a `try/except` block around the error to properly handle tree names that return 404 from LandoAPI and other errors.
…(Bug 1897044) (#204) The `error` and `exception` level logging calls cause Sentry to treat expected states as new issues. Lower the level to `info` to quiet down Sentry. Issues experienced by the Treestatus API will still show up in the LandoAPI logs, and at the `info` level we can still debug which code paths are taken by various requests.
…rs (Bug 1901484) (#205) Add a new `is_treestatus_user` template helper which checks for membership in the Treestatus Mozillians groups. Switch to using this new function to determine if Treestatus editing elements of the UI should be shown.
|
Here is the Lando-UI merge commit. I've commented out the Flask-based Treestatus code for now, it was causing some issues in tests and since porting it is out of scope I figure I could just un-comment it when we do the full port to Django. If there's another way to do this that would be preferred I am happy to go down that path. :) |
What is the plan for merging this PR? Is it to squash and merge it (and lose the commit history) or keep the merge commits? As discussed on Slack, it would be easiest to do this in the same way we did it for the API merge (i.e., one merge commit for the merge branch which includes very few conflicting files, and one squashed commit for manual fixes to get things to actually work, which can include commenting out the treestatus stuff). The current branch has a mix of the merge commits and a bunch of fixup commits that we will have to squash anyway before merging, separately from the original merge, so I still think this would be better in two PRs. If we wanna keep it in one PR we should squash the additional commits into their own commit on top of the merge commit. |
There was a problem hiding this comment.
Left some comments, I think ideally we will treat this PR like we did the API PRs, i.e., split it into the merge PR and fixup PR. However, since it is a fairly small number of commits, we could also squash and merge the entire PR and lose the history.
I mostly left the treestatus code unreviewed, though there are some requested changes that are functional (e.g., in some of the templates) -- I think this should be manually tested before the last review to make sure the UI works as expected.
I'm fine with squashing and merging the entire PR without worrying about the history, if that makes things easier. Many of the finer details in those commits are no longer relevant since they are related to accepting forms via Flask.
Yes, I plan to deploy it to dev and run through a landing to make sure everything working as expected. |
| {% set blocker = dryrun['blockers'][0] if 'blockers' in dryrun else dryrun['blocker'] %} | ||
| <h3 class="StackPage-landingPreview-sectionLabel">Landing is Blocked</h3> | ||
| <div class="StackPage-landingPreview-section StackPage-landingPreview-blocker"> | ||
| {{ dryrun['blocker']|escape_html|linkify_faq|safe }} | ||
| {{ blocker|escape_html|linkify_faq|safe }} |
There was a problem hiding this comment.
It might be better to do add blocker to blockers and loop over blockers rather than showing blockers[0] only?
| @@ -1,4 +1,5 @@ | |||
| {% if revision['blocked_reason'] %} | |||
| {% if revision['blocked_reasons'] or revision['blocked_reason'] %} | |||
| {% set blocked_reason = revision['blocked_reasons'][0] if 'blocked_reasons' in revision else revision['blocked_reason']%} | |||
If anything, I'd be in favour of something that keep the commits that have more details in their message independent, as this provides context that might be worth preserving in the main history. |
No description provided.