Fix bug: incorrect author link hidden filter in combined search

[demiankatz]

This PR is intended to fix a bug where hidden filters from the last combined search column get inappropriately applied to author links in earlier columns.

TODO

• Add test to demonstrate bug
• Add mechanism for persisting search IDs in record drivers
• Fix bug

[demiankatz]

Update: I haven't found a fix yet, but I have found a workaround. I've updated the test to try different combinations of AJAX settings for the columns. The test passes if both columns (or just the left column) are ajax=true. It fails if both columns (or just the left column) are ajax=false. So until a fix is found, you can turn on the ajax setting for some or all of your columns to work around the problem.

[EreMaijala]

At the moment the best solution I can think of is to allow record drivers to have access to Params of current search (the active search or the one from the sid parameter) so that they'd have access to actual current filters. Maybe this could be done in a back-compatible manner so that it falls back to search memory if Params is not there. In the long run this would simplify getting hidden filters.

[demiankatz]

At the moment the best solution I can think of is to allow record drivers to have access to Params of current search (the active search or the one from the sid parameter) so that they'd have access to actual current filters. Maybe this could be done in a back-compatible manner so that it falls back to search memory if Params is not there. In the long run this would simplify getting hidden filters.

Which record driver method(s) would need access to the Params? I'm currently working to decouple record drivers from database logic, because I think it would be better if they were purely data models instead of containing so much business logic. Adding Params access seems like it might be a step counter to that principle. Is it possible that we need a helper class that takes a record driver and applies Params-related logic, rather than embedding this in the record driver itself?

[EreMaijala]

@demiankatz You're right about that. As discussed elsewhere, storing the search ID with the record driver would be a better way, and it would be possible to implement in a back-compatible way.

[demiankatz]

Unfortunately, I think it's getting a bit late to finish this in time for 9.1 or 10.0, so I'm pushing this milestone to 10.0.1.

[demiankatz]

(If a miracle occurs and this gets done early, I'd of course still be happy to merge it... but I just don't think that's likely).

[demiankatz]

@EreMaijala, as discussed on today's Community Call, I went ahead and added a mechanism to store the search ID in the record driver. Since we're aiming to include this fix in a point release, I opted for the simplest approach -- using the existing setExtraDetails method instead of building new purpose-built getters/setters for search IDs. I'm open to revisiting this decision now or in the future, but this seemed like a very unintrusive approach and thus a viable starting point.

Please let me know what you think, and if you'd like me to attempt any further work on this. (This part was considerably less complicated than I had anticipated).

[EreMaijala]

@demiankatz I finally got around to fixing this. Should work with the latest commit, but I can't ask for review from you in your own PR.

[demiankatz]

Thanks, @EreMaijala, looks good to me. I'm running tests and expect them to pass. Do you mind approving this so I can merge it (assuming nothing unexpected occurs)?

[demiankatz]

(Tests are passing, as expected).