-
Notifications
You must be signed in to change notification settings - Fork 25
Overhaul of query handling — q
permalinks, session[:query_record]
#3212
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
Merged
+1,519
−1,553
Conversation
This file contains hidden or 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
Also convert `pass_query_params` ivar @query_params to @query_param (from hash to string)
…sion-query-record Switch to use `session[:query_record]`. Turn off long permalinks where not needed to reproduce query results
q
permalinks, session[:query_record]
…sion-query-record Switch from `add_query_param` to `add_q_param` etc
JoeCohen
reviewed
Aug 29, 2025
Crashing bug:
|
The crashing bug appears to be specific to the "Go" button. The left and right arrow buttons appear to work fine. |
Do not pass the :id through to next/prev page.
Letters form should use the same
Use Set in case there are doubles, as for many Genus names
Test for concise filters
@mo-nathan After several false starts, i actually have the "go to page" form fixed. I added tests checking the url format and making sure the Also added |
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.
session[:query_record]
This PR totally changes how MO keeps track of a user's current query, which is how they can step through results, see associated records, map them, etc. The big change is storing the
QueryRecord
id
in the session, rather than manually passing the alphabetizedid
through the url as aq
param, as we currently do. TheQueryRecord
is created when the query is first executed (e.g., by an:index
), and is thereafter available without being manually passed.The issue with the current approach is that the
QueryRecord
is perishable (like a session), but its ID is encoded in the URL like a permalink. It currently has to be passed through the url so that ashow
page, for example, can know what the previous/next records were, or what filtered index to go back to.But the obvious place to store a reference that needs to be ubiquitously available, like the current user, is the
session
(i.e. a cookie).q
permalinksFor links or requests that need to modify the query (a search or some other filtered link), they can now send a
q
param in the URL that will function as a shareable permalink.On the coding side, with this change we can now build a query simply by writing a params hash for
q
into a link. The params and values obviously have to be params for the relevantQuery
class, but these are all documented. The following will produce a permalink and the desired results.No need to resort to “pattern search” permalinks.
Details:
q
param for most urls. Only on indexes, maps, downloads, etc. - pages that are the results of queries. Theq
param in those cases will be a Rails-encoded hash that describes the query, and is used by the receiving action to generate the results. The hash is barely legible, but the important thing is that it's parseable by Rails without us doing anything. If you callparams[:q]
in the receiving action, you will get the exact hash you sent, with the keys already symbolized.q
param, either as a result of a search or a permalink, the index action saves a description of the query in aQueryRecord
, as currently. The change is that the record ID is stored insession[:query_record]
so it doesn’t have to be passed in the URL. Any action in the app has access to the session, so they have access to the sameQueryRecord
previously passed in theq
param. (Why didn’t we do this before?) Tons and tons of repetitive code has been deleted.pass_query_params
, anywhere. Far fewer calls toadd_query_param
,redirect_with_query
,link_with_query
,icon_link_with_query
etc.q
params. Incomingq=34iuy
params will get immediately updated to permalinks before being passed to the next page, so they will get phased out pretty quickly. Tested.permalink
column to QueryRecord that we can write if the record's URL is now a permalink. QueryRecords are updated every time the alphabetizedq
hits a page - that's how we know if they're still in use or whether to delete them. WhenQueryRecord.where(permalink: false).count == 0
and is stable for awhile, we can delete the code that handles alphabetizedq
.q
hash in the session directly, rather than a reference to the QueryRecord? Because the encoded hash might exceed the allowable size of a cookie.QueryRecord
mechanism intact, because that's how we could store multiple queries per user, e.g. a Names query, an Observations query, or several of each. Outside the present scope