Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/deprecate tool #1404

Merged
merged 21 commits into from
Dec 17, 2024
Merged

Feature/deprecate tool #1404

merged 21 commits into from
Dec 17, 2024

Conversation

michaeljcollinsuk
Copy link
Contributor

@michaeljcollinsuk michaeljcollinsuk commented Dec 4, 2024

📝 Summary

This PR relates to ministryofjustice/analytical-platform#6111

It allows AP admins to:

  • Mark a tool as deprecated
  • Optionally add a deprecation message to display to users. Otherwise a default message is used
  • When a user selects a deprecated tool, they can still deploy it, but the deprecation message is displayed
  • Retire a tool, to make it inaccessible to users. A message is displayed to users telling them they must deploy a different version

Furthermore, it adds image_tag as a field to the Tool model and makes it a required field when creating a new tool release. This helps simplify the code elsewhere when looking up what version of a tool a user has deployed.

There are a number of other minor related changes including:

  • Tighten up the logic that disables the "Open", "Deploy" and "Redeploy" buttons based on the tool the user has selected
  • Add a status to tools, and display this in the admin tool release page
  • Update tool release filtering to filter by these statuses
  • Remove the code which would get details about a release from the helm chart directly. This caused the tool page to be very slow to load, and by making the image_tag and description fields on a Tool release required fields, is no longer necessary.

Merging this PR will have the following side-effects:

  • Once merged, it will be difficult to rollback these changes.
    Screenshot 2024-12-13 at 16 17 24
    Screenshot 2024-12-13 at 16 17 10
    Screenshot 2024-12-13 at 16 16 58
    Screenshot 2024-12-13 at 16 15 37

🔍 What should the reviewer concentrate on?

  • Any feedback on the code

🧑‍💻 How should the reviewer test these changes?

  • Pull down the branch
  • Migrate the database
  • Try creating some releases, and marking them as deprecated, to the behaviour when a deprecated tool is selected
  • Try deploying a deprecated release
  • Then make the release retired, go back to the "Your tools" page and check what is displayed
  • Switch between tool options to test the scenarios for when the open/deploy/restart buttons are disabled.
  • The "Open" and "Restart" buttons should only be enabled when there is an installed tool deployed and it has been selected in the tool dropdown list
  • The "Deploy" button should only be enabled when selecting a tool that is not currently installed

📚 Documentation status

  • No changes to the documentation are required
  • This PR includes all relevant documentation
  • Documentation will be added in the future because ... (see issue #...)

@jamesstottmoj
Copy link
Contributor

Tested locally. Works well. Needs updating to latest main but otherwise LGTM

jamesstottmoj
jamesstottmoj previously approved these changes Dec 17, 2024
Copy link
Contributor

@jamesstottmoj jamesstottmoj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

- If a tool is depcrecated, show the deprecation message when it is selected
- If a tool is retired, hide it and show a message that user must upgrade
If a deployed release is selected, enable open and restart button.
If a undeployed release is selected, enable deploy button only.
Use the value stored in the DB when deploying the tool
Previously all restricted tools were excluded. However, allowing tools
to be deprecated means we can update this logic. As some restricted tools
may not be deprecated, so should not display an "unsupported" message.
@michaeljcollinsuk michaeljcollinsuk merged commit 7044c8b into main Dec 17, 2024
12 checks passed
@michaeljcollinsuk michaeljcollinsuk deleted the feature/deprecate-tool branch December 17, 2024 09:53
Copy link

sentry-io bot commented Dec 17, 2024

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ MaxRetryError: HTTPSConnectionPool(host='6f58d1c4fd8fae6d3d9282a400edce79.gr7.eu-west-1.eks.amazonaws.com', port... /tools/ View Issue

Did you find this useful? React with a 👍 or 👎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants