Skip to content

Conversation

@felixarntz
Copy link
Member

Summary

Fixes #737

Relevant technical choices

  • An admin notice is added with 3 steps to migrate, each of which is conditionally marked already completed depending where the site owner is at in the process.
  • An admin pointer is added as an additional way to make site owners aware that they need to take an action, since admin notices can easily be overlooked across other admin notices.
  • A few minor interoperability fixes we added (in the modules/database/sqlite files).
  • No new functions are being introduced since they would need to be removed again in the following release. Therefore the new temporary code relies heavily on closures.
  • Also see the requirements on the issue.

Screenshots

Admin notice awaiting step 1
Screenshot 2023-05-31 at 3 57 35 PM

Admin notice awaiting step 2
Screenshot 2023-05-31 at 3 59 29 PM

Admin notice awaiting step 3
Screenshot 2023-05-31 at 4 05 01 PM

Admin pointer
Screenshot 2023-05-31 at 3 06 46 PM

Checklist

  • PR has either [Focus] or Infrastructure label.
  • PR has a [Type] label.
  • PR has a milestone or the no milestone label.

@felixarntz felixarntz added [Type] Enhancement A suggestion for improvement of an existing feature [Focus] Database labels May 31, 2023
@felixarntz felixarntz added this to the 2.4.0 milestone May 31, 2023
Copy link
Member

@mukeshpanchal27 mukeshpanchal27 left a comment

Choose a reason for hiding this comment

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

Thanks, @felixarntz. It looks good to me. I have some nitpick feedback and a few questions.

Sometimes, when we activate the SQLite Database Integration through step 2 and then return to the Performance Lab settings page, deactivating the module also deactivates the SQLite Database Integration plugin. is it intended behaviour?

@felixarntz
Copy link
Member Author

@mukeshpanchal27

Sometimes, when we activate the SQLite Database Integration through step 2 and then return to the Performance Lab settings page, deactivating the module also deactivates the SQLite Database Integration plugin. is it intended behaviour?

I noticed that too, that's okay. It is due to the migration routine that the standalone plugin comes with. In the cases where the plugin takes care of that automatically, even better, one less step for the user.

Copy link
Member

@mukeshpanchal27 mukeshpanchal27 left a comment

Choose a reason for hiding this comment

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

Thanks @felixarntz, LGTM!

@felixarntz
Copy link
Member Author

@aristath @joemcgill Would one of you be able to review this PR please?

Copy link
Member

@aristath aristath left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Type] Enhancement A suggestion for improvement of an existing feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement notice to prompt administrator to update to SQLite plugin instead of SQLite module

4 participants