DEV: Split the Query Listing and Query Editing code #356
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
martin-brennan
approved these changes
Feb 10, 2025
app/serializers/discourse_data_explorer/query_details_serializer.rb
Outdated
Show resolved
Hide resolved
assets/javascripts/discourse/templates/admin/plugins-explorer-index.hbs
Outdated
Show resolved
Hide resolved
|
Thank you!
…On Mon, Feb 10, 2025 at 1:23 PM Gary Pendergast ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In config/locales/server.en.yml
<#356 (comment)>
:
> @@ -23,7 +23,7 @@ en:
Here are the results:
%{table}
- <a href='%{base_url}/admin/plugins/explorer?id=%{query_id}'>View query in Data Explorer</a>
+ <a href='%{base_url}/admin/plugins/explorer/queries/%{query_id}'>View query in Data Explorer</a>
Yep, it was only client side, so I've added a redirect there.
—
Reply to this email directly, view it on GitHub
<#356 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAHAXAABCKYP2XEDZOHPRJ32PALRVAVCNFSM6AAAAABWVFLNJWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDMMBUGYZDEMBTGI>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
✨ What's This?
The code for listing all of the defined queries is mixed together with the code for editing a single query. Notably, this results in large amounts of unnecessary data being loaded for the list view, which causes substantial rendering slowdowns.
To address this issue, we now only load the necessary data for the list view, and load the full data when it's actually needed (any endpoint that returns a single query). The primary changes that achieve this are:
QueryDetailsSerializerserialiser, which includes all of the query info, and change the existingQuerySerializerserialiser to only include the necessary attributes of each query for generating a list of them all./plugins/explorerroute into/plugins/explorerfor showing just the list of queries, and/plugins/explorer/queries/:query_id, for showing/editing/running a specific query.👑 Testing
Check that plugin functionality works as expected. I've gone through everything I could find, but I'm not 100% familiar with expected behaviour, so may've missed something.
Review the acceptance tests and system specs, to confirm that there are no missing uses cases.