search

LibriVox / librivox-catalog

LibriVox catalog and reader workflow application
https://librivox.org
MIT License
36 stars 17 forks source link

Apply the double-search change to simple search #208

Closed redrun45 closed 3 months ago

redrun45 commented 3 months ago

...and also add some comments for visibility.

notartom commented 3 months ago

OK, I think it had to do with a config difference between prod and staging/localdev. By enabling error logging (log_threshold=1) on prod, I could see the following error logged multiple times:

ERROR - 2024-03-17 16:18:29 --> Severity: Warning --> Undefined array key "full_count" /librivox/www/librivox.org/catalog/application/controllers/catalog/Search.php 183

When I merged and pulled this PR, those stopped.

redrun45 commented 3 months ago

By enabling error logging (log_threshold=1) on prod, I could see the following error logged multiple times:

I see! What I was seeing on localdev is that the page never switched to a new URL - it just stayed on whatever page you started the search from, which was why I couldn't just give an example. With logging on, I suppose the server goes ahead and sends the AJAX response, instead of the HTML error message I was seeing.

That way, if you're proposing a PR that isn't ready yet, you can disapprove your own PR, and then change your approval once it's ready.

Sorry to put you to the extra trouble, but this sounds good. I don't do anything like this at my day job, so having some guard rails that force me to be 100% clear and matching your expectations sounds like the way to go.