Add ability to filter message recipients by activity

[mathjazz]

Fix #3244.

This is a continuation of what was started in #3314. It implements the remaining bits of the Compose screen in the Messaging Center, namely Activity filters. As mentioned in #l10n-team last Friday, Activity filters are always visible, which is not what the spec says, but it’s simpler from the UX and codebase point of view.

The diff looks bigger than the changes actually are because of a dedent in Remove redundand condition and formatting of the HTML file in Update check-box and messaging styling. Hence, I suggest reviewing commit by commit.

Deployed to stage:
https://mozilla-pontoon-staging.herokuapp.com/messaging/

[eemeli]

Selecting "Hide whitespace" from the diff view options got rid of the indent noise.

[eemeli]

This function seems to perform an excessive number of DB queries, but it's probably okay to leave this unoptimized given how rarely it'll be called.

[mathjazz]

Forgot to mention that the queries are very slow, and might time out. I'll work on that before unlocking the ability to actually send email to the selected users (not to yourself).

[eemeli]

Testing the functionality on stage, I'm getting a 503 response from trying to send a test notification. Is that intentional?

[bcolsson]

I'm less concerned about the activity filters being shown, but right now it seems like activity filters are always going to be enabled. Even if number of translations min/max of 0/0 means any number of translations, the date filter is still restricted to within the last year.

At the bare minimum can we include an enabled checkbox (with the default being unchecked)?

[mathjazz]

but right now it seems like activity filters are always going to be enabled. Even if number of translations min/max of 0/0 means any number of translations, the date filter is still restricted to within the last year.

The default date values can be deleted and then activity filters aren't applied (which is the same as unchecking the box).

Should that (no value set) be the default behavior?

See the example for Translation filters below:

[bcolsson]

but right now it seems like activity filters are always going to be enabled. Even if number of translations min/max of 0/0 means any number of translations, the date filter is still restricted to within the last year.

The default date values can be deleted and then activity filters aren't applied (which is the same as unchecking the box).

Should that (no value set) be the default behavior?

The likelihood of someone wanting all 3 filters applied is low, the likelihood of someone forgetting to delete default values if they don't want a filter enabled is high. Testing it out, to actually delete the date from the box I have to:
click, delete, tab, delete, tab, delete
That's for just 1 field, so I have to do that 6 times if I don't want to have any activity filters?

We could remove the defaults to keep everything empty, but I feel like clearly indicating whether an activity filter is enabled or not seems like a better UI

[mathjazz]

The likelihood of someone wanting all 3 filters applied is low, the likelihood of someone forgetting to delete default values if they don't want a filter enabled is high. Testing it out, to actually delete the date from the box I have to: click, delete, tab, delete, tab, delete That's for just 1 field, so I have to do that 6 times if I don't want to have any activity filters?

You don't need to do that if the dates are not set by default. 🤣

But yes, the default browser date field seems to work the way you describe it...

We could remove the defaults to keep everything empty, but I feel like clearly indicating whether an activity filter is enabled or not seems like a better UI

I 100% agree this is not an ideal UX.

But while I understand this feature is critical, it's also going to be used very rarely by very few people. And only after we start using it, we'll understand how it's going to be used.

So my suggestion is to get it to a point where it’s functional and solves the main problem, and then we can circle back to it after we've collected some usage data and delivered other features.

[bcolsson]

Alright, then the least bad option here would be to have no default values here then.
It seems like we're not doing validation- but I suppose not a big issue (the only would be sending to 0 people if the min is higher than the max for number of translations).

I'll approve now to not block, but would like default values removed from dates.

[bcolsson]

Approved (assuming defaults are removed from the From / To date filters.

[mathjazz]

It seems like we're not doing validation- but I suppose not a big issue (the only would be sending to 0 people if the min is higher than the max for number of translations).

The input is validated as per the spec. We could add additional validation for cases where Maximum < Minimum or To < From, but I'd leave that for later. As you mention, in cases like this we'll not send a message to any users anyways, which is also a validation.