Use normalizes on CustomFilter#context value

[mjankowski]

This was originally in #27521 but after the final Rails 7.1 rebase there were some errors that I couldn't figure out quickly so I removed it from that one.

Added some spec coverage here to be more confident and I think this change (adding normalization, refactor on existing validation) preserves the prior validations and data cleanup behaviour. Will watch CI to see if the prior issue is resolved.

[mjankowski]

Hmm, error still present ... looks like somewhere in caching steps there's a proc where there used to not be a proc after this change. Will review.

[mjankowski]

Somewhere in CustomFilter.cached_filters_for during the Rails.cache.fetch block something is attempting to be Marshalled which cannot be b/c its a proc. This is definitely breaking because of the normalizes call, and doesn't seem to be limited to just the context (array column) but also happens if other columns are normalized (I tried a simple one with title to eliminate the array column being an issue).

I'm able to replicate in console, so its not specific to spec setup/mocks/etc.

Might try to produce minimum replication to see if its framework feature issue or something with how we are caching.

[mjankowski]

This is a limitation of the normalizes feature ... but will resolve itself when we update the marshalling format - rails/rails#49871 - as part of 7.1 config updates.

[mjankowski]

Rebased this and updated the marshalling config, I think these specs will pass now. However:

• As noted in a TODO comment here, there's probably a loading interaction happening with some gem we are using which is making the normal way to set the config value not work (see linked issue).
• We should probably contemplate a strategy for updating the marshalling format and how it might interact with rolling deploys and so on. I think it's like the others where it's safe to do an initial Rails 7.1 deploy which keeps the old format, and then enable the config on a subsequent deploy and that will be safe.

All that said, even if this passes CI, hold off on merge until we resolve all that.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.01%. Comparing base (86500e3) to head (f31a038).
Report is 241 commits behind head on main.

❗ Current head f31a038 differs from pull request most recent head 589f9b2. Consider uploading reports for the commit 589f9b2 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #27602      +/-   ##
==========================================
- Coverage   85.01%   85.01%   -0.01%     
==========================================
  Files        1059     1060       +1     
  Lines       28277    28283       +6     
  Branches     4538     4537       -1     
==========================================
+ Hits        24040    24044       +4     
- Misses       3074     3075       +1     
- Partials     1163     1164       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[mjankowski]

Rebased this after the linked PR added the validation specs and small refactor.

This is now just the addition of normalization specs, the marshalling format change, and replacing previous method with normalizes usage.

I think as long as we update the marshalling format either from this PR or elsewhere, this will be safe at this point.

[mjankowski]

Rebased against main after another PR updated the marshalling format to 7.1

[mjankowski]

A previous rebase here hit what I think is an intermittent failure - https://github.com/mastodon/mastodon/actions/runs/7427240060/job/20212618059?pr=27602

Rebased again, and the latest run seems fine. May attempt to track down that failure separately.

[ClearlyClaire]

This drops the Array() around context, but this is probably fine since passing a singular value is already rejected at the controller level as far as I can tell.