Before, when the "Add new champ" button was clicked, the new champ
was inserted after the **first** fully visible champ.
That was most of the time unexpected. The correct behavior would be to
insert the new champ after the **last** fully visible champ.
That's what this commit does. Now the "Add new champ" behavior feels
much less confusing.
it follows service-public.fr structure
- remove extraneous footer-link class
- remove extraneous space with `>` haml weird stuff
- homogenize json-style hash
- merge classes on ul
- use pseudo element for eye candy
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.
When inviting an instructeur, the code first looked up an existing
instructeur using the normalized (downcased) email address–but then
tried to promote it using the non-normalized version, which failed.
This fixes the issue by always using the normalized email.
Before the form attempted to read an email value from the Instructeur
model, and failed (because the empty Instructeur had no user yet).
We could let `Instructeur#email` return `nil` if there is no User –
but as a created Instructeur is always supposed to have a User, this
seems like a nice safeguard to keep.
So instead this commit rewrites the create form, which now doesn’t
depend on an Instructeur model. Seems easy enough for now.
It kind of worked until now, because the email field is disabled, and
thus never accessed.
But better make it clean, by accessing an object (User) where the email
field actually exists.
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.
