Skip to content

Conversation

nimmolo
Copy link
Contributor

@nimmolo nimmolo commented Aug 21, 2025

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 alphabetized id through the url as a q param, as we currently do. The QueryRecord 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 a show 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 permalinks

For 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 relevant Query class, but these are all documented. The following will produce a permalink and the desired results.

link_to(
  observations_path(
    params: { 
      q: { model: :Observation, 
           by_users: [1],  locations: [720], 
           names: { lookup: "Hygrocybe felix", include_synonyms: true }, 
           projects: [330], order_by: "confidence" }
    } 
  )
)

No need to resort to “pattern search” permalinks.

Details:

  • MO will no longer have a q param for most urls. Only on indexes, maps, downloads, etc. - pages that are the results of queries. The q 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 call params[:q] in the receiving action, you will get the exact hash you sent, with the keys already symbolized.
  • When a user requests an index page with a q param, either as a result of a search or a permalink, the index action saves a description of the query in a QueryRecord, as currently. The change is that the record ID is stored in session[: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 same QueryRecord previously passed in the q param. (Why didn’t we do this before?) Tons and tons of repetitive code has been deleted.
  • No more pass_query_params, anywhere. Far fewer calls to add_query_param, redirect_with_query, link_with_query, icon_link_with_query etc.
  • For backward compatibility, indexes can still handle incoming "alphabetized" q params. Incoming q=34iuy params will get immediately updated to permalinks before being passed to the next page, so they will get phased out pretty quickly. Tested.
  • Adds a temporary permalink column to QueryRecord that we can write if the record's URL is now a permalink. QueryRecords are updated every time the alphabetized q hits a page - that's how we know if they're still in use or whether to delete them. When QueryRecord.where(permalink: false).count == 0 and is stable for awhile, we can delete the code that handles alphabetized q.
  • Why not just store the 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.
  • I also want to keep the 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

nimmolo added 30 commits August 21, 2025 03:16
ImagesControllerTest#test_destroy_image_with_query demonstrates what's wrong with encoding. `q` is getting double-encoded as a query string when generated from the `@query_params` variable.
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
@nimmolo nimmolo changed the title q permalinks Overhaul of query handling — q permalinks, session[:query_record] Aug 29, 2025
@nimmolo nimmolo requested a review from mo-nathan August 29, 2025 19:33
@mo-nathan
Copy link
Member

Crashing bug:

  1. From the home page click Search.
  2. Search for “Amanita”.
  3. Click on an observation.
  4. Click on the Observations Index icon.
  5. Enter “10" in the pages.
  6. Click “Go” icon.
  7. Boom!
TypeError in ObservationsController#index
no implicit conversion of Symbol into Integer
Extracted source (around line #240):

  def query_from_q_param_hash
    q_param = params[:q]
    return nil if q_param[:model].blank? # <=== Failing line

    Query.lookup(q_param[:model].to_sym,
                 **q_param.except(:model).to_unsafe_hash)

Rails.root: /Users/nathan/src/mushroom-observer

Application Trace | Framework Trace | Full Trace
app/controllers/application_controller/queries.rb:240:in `[]'
app/controllers/application_controller/queries.rb:240:in `query_from_q_param_hash'
app/controllers/application_controller/queries.rb:215:in `query_from_q_param'
app/controllers/application_controller/indexes.rb:176:in `order_by_or_flash_if_unknown'
app/controllers/application_controller/indexes.rb:168:in `sorted_index_opts'
app/controllers/application_controller/indexes.rb:154:in `sorted_index'
app/controllers/application_controller/indexes.rb:53:in `block in build_index_with_query'
app/controllers/application_controller/indexes.rb:49:in `each'
app/controllers/application_controller/indexes.rb:49:in `build_index_with_query'
app/controllers/observations_controller/index.rb:7:in `index'
app/controllers/application_controller.rb:148:in `catch_errors_and_log_request_stats'

@mo-nathan
Copy link
Member

The crashing bug appears to be specific to the "Go" button. The left and right arrow buttons appear to work fine.

@nimmolo
Copy link
Contributor Author

nimmolo commented Aug 31, 2025

@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 :id param is not there on next/prev page links.

Also added q_param as a method of a Query instance.

@nimmolo nimmolo merged commit 550cfd2 into main Aug 31, 2025
9 of 11 checks passed
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.

4 participants