client_key is exposed to the client via gon, so if we use it for sending email too we are exposing a key so anybody could send an email.
The current client_key has a different level of right and can't send emails so it's ok to expose it.
The `joins` are declared explicitely in order to associate a predictable
name to the joined table.
Otherwise, when the query is joined with `:users`, ActiveRecord will
alias the join automatically to solve the conflict. Unfortunately, the
automatic resolution means that the table name becomes unpredictable,
and thus unsuitable to perform queries on.
Now that `Instructeur.email` is merely an alias to `instructeur.user.email`,
and we changed every occurence of `instructeurs.pluck(:email)` to
`instructeurs.map(&:email)`, the new version using `map` may cause N+1 queries
if the users have not been preloaded.
It makes sense to always preload the user when fetching an Instructeur:
- Instructeur and User have a strongly coupled relationship
- It avoids N+1 queries everywhere in the app
Of course fetching an instructeur without needing its user will now do an
unecessary fetch of the associated user. But it seems better than leaving
a risk of N+1 queries in many places.
- rename `dossiers_id_with_notifications` to `followed_dossiers_with_notifications`,
- rename `notifications_per_procedure` to `procedures_with_notifications`,
- return an ActiveRecord::Relation instead of the result of the query, so that the call place can compose it,
- `merge` with the wanted Dossier scope in the call places, don’t bother passing it as a parameter,
- use the “state” (now “scope”) parameter as a scope method that can be just applied on `Dossier`.
This is used in /procedures#show and /procedures#index, to display badges on the “suivis” and “traités” tabs of each procedure. Rails cache helps when it’s the exact same query, but it’s not the case for different tabs.
I’m not certain it’ll be a visible performance improvement but it shouldn’t hurt.
