Default filters for anonymous users

[MoshiKoi]

Hopefully resolves #1043

Proof of concept, hasn't been fully tested yet.

For anon users, it defaults to the category default
For signed-in users, it defaults first to their default if it exists, then to the category default

I put the default filter setting with the rest of the category settings, so only admins can edit it.

[MoshiKoi]

Note: https://meta.codidact.com/posts/288886 is still an issue with this, which is really bad because users cannot change the category filter.

[MoshiKoi]

The above issue has been hopefully fixed with the last commit by adding an explicit System "None" option to the seeds.

[trichoplax]

Looks good.

Tested in local development environment:

• Default for category is applied and appropriate message shows:
  • When no user is signed in.
  • When a user is signed in with no user default for that category.
• Default for user is applied when present and signed in.
• Default can be overridden to "None":
  • When it is the category default and there is no user signed in.
  • When it is the category default and there is a user signed in.
  • When it is the user default.
• "None" can be cleared, resulting in the correct default being restored.

Deploying to production

Note that I ran the following in order to add the new "None" filter and to get it to show up in the user interface:

rails db:seed
redis-cli flushall

That second one clears all of the redis cache, so causes all the new activity indicator circles to appear on all categories. This is not ideal for deploying to production. Is there a more precise approach that will just clear the cache aspect relating to the filters?

[cellio]

Clearing the whole redis cache is bad (annoys everyone). If we can't selectively flush just the part we need, what happens here if we don't flush it at all? Is there a way to instead push in the changes we want, without flushing stuff we don't want to lose?

[trichoplax]

Very agreed that clearing the whole cache is to be avoided if at all possible. I don't know a way of pushing in just the changes we need, but it seems likely that this would be possible - hopefully someone else will be able to tell us how?

In the meantime, could we avoid merging this one so it doesn't hold up deploys? Is there anything we can change about it to prevent accidentally merging?

The merge is currently blocked by merge conflicts but I'm not sure whether resolving those allows immediate merging or requires another review (that is, I'm not sure if the repo is configured to insist on a new review after any new commit).

[trichoplax]

@MoshiKoi This has merge conflicts preventing it from being merged into develop. If resolving those results in it asking for another review, feel free to ping me.

[trichoplax]

@cellio I just realised I didn't answer the other part of your question:

At least in my local development environment, not clearing the cache means that the predefined filters have no "None" option. This means that pressing "Clear" clears the filter back to the category or user default, so there is no way of applying no filters. An insightful user could create their own new filter which behaves the same way as "None", but I wouldn't want to depend on that as a workaround, especially as it is not an option when not signed in. I'd personally prefer that we not have default filters until we also have the "None" filter.

[Taeir]

Clearing the whole redis cache is bad (annoys everyone). If we can't selectively flush just the part we need, what happens here if we don't flush it at all? Is there a way to instead push in the changes we want, without flushing stuff we don't want to lose?

If this is a system filter, then we should be able to just clear the key "system_filters" (which we should perhaps have expire automatically after some amount of time to periodically refresh it anyway?)

Relevant code is in users_controller#filters_json

To clear only the system filters, run the following in a rails console (or add as a migration AFTER adding the new filter)

Community.all.each do |c|
  RequestContext.community = c
  Rails.cache.delete('system_filters')
end

[Taeir]

If we change the first line of users_controller.rb#filters_json (~line 83) to the following:

system_filters = Rails.cache.fetch 'default_system_filters', expires_in: 1.day do

Then it will recompute the system filters every day. I changed the name of the key (otherwise it will never apply the expiry) and added the expires_in part.

[trichoplax]

Since this pull request is from a fork, I don't think anyone but MoshiKoi can push commits to it. To work around this I've changed the base branch of this pull request to a new branch of the same name on the main QPixel repository. My hope is that this will allow continuing work with Taeir's cache fix on the new branch, which can then be merged into develop with a new pull request.

I haven't yet merged this, for 2 reasons:

1. I'd like a second opinion on whether changing to a new base branch is a good approach.
2. There is a merge conflict in db/schema.rb. Since the only conflict is the schema version date my guess is that it's fine to just take the most recent of the 2 dates, but I'd like a second opinion on this too.

[cellio]

@Taeir I believe you told me that when the only difference in db/schema.rb is the date, we should take the later and resolve the conflict. Can you confirm? Also, can you weigh in on the first question in the previous comment? Thanks.

[Taeir]

@cellio that is correct.
@trichoplax uts ticked by default when you make a pr that contributors can make changes to your code. I think that should allow directly pushing to the branch that lives in our repo (moshi/…) rather than in moshis repo (…)

[trichoplax]

@Taeir thanks for explaining that we can still make commits even though the branch is on MoshiKoi's fork. I have therefore changed the base branch back to develop.

I have now merged in develop, taking the latest date in db/schema.rb to resolve the conflict as you advised.

Since we'll now be able to make your suggested cache change on this branch, I'll try to do that when I'm next at a computer.

I have made a further commit so I'd like someone else to review this.

[trichoplax]

I've made @Taeir's suggested change so that the system filters cache will be refreshed without needing to delete any other caches. This seems like a good idea to me, but I am not yet familiar with how Rails caches work, and have run into problems trying to understand them in other contexts. For this reason I'd like this to be reviewed by someone else. Also, I generally try to avoid reviewing anything I have made a commit to.

[cellio]

@ArtOfCode- can you take a look at the rails question in the previous comment?