Fetch and display commune name

[Morendil]

Purpose

New users from communes should be able to make sense of what they can do with Régie.

Proposal

When arriving at Régie from ProConnect, make sure things are set up properly:

• lookup the Commune using the provided SIRET
• ensure the Organization is created with the commune's name

Fixes #610

[Morendil]

@PanchoutNathan On est pas dans les clous par rapport à la maquette: est-ce que tu aurais le temps (genre dans la semaine) de committer ou suggérer ce qu'il faudrait faire ? Sinon on peut merger et corriger plus tard… Thx 🙏

[mjeammet]

Nice ! Let's try and wait for Nathan for a few days if per chance he can adjust the design of the button by the end of the week. Otherwise, let's gooo !

[Morendil]

@mjeammet This failed in CI for an interesting reason - the API is rate-limited and we got a 429 error. I don't know how we managed to exceed 7 calls/sec. with only one E2E test exercising it… Not sure how best to proceed - perhaps implement retry logic, or try a different API, or use static data… it raises the question of how to deal with failure cases for something like this where the request is made at a specific point in the lifetime of an organization, and not retried afterward.

[Morendil]

I have opted to implement retry logic, and merge with no further delay…

[qbey]

This should not be enforced here because this is not generic.

[Morendil]

General reply to your comments: thanks for the feedbacks, I've already hit "merge" but can address these in a later PR (might pick #622 which is in some ways a continuation, or a new one altogether depending on urgency).

Specific reply to the above: OK, I understand the concern here is for users and contributors outside of France to be able to start from a vanilla configuration instead of having to figure out what is French-specific (or even French territorial administration specific) stuff that needs swapping out. And I don't think this case has arisen before so it's a good occasion to start documenting that.

On the other hand I'd like to keep these active for local development and documented in the repo for ease of onboarding. Can we put these in, say, env.d/development/france.dist and pick them up in docker-compose.yml with a comment suggesting creating a country-specific set of env vars, as needed for other countries ?

[qbey]

Same here, this should not be enforced here, only in specific tests.

[qbey]

I don't understand the condition, why should it be only when there is a registration ID?

[Morendil]

That does seem indirect now you mention it, testing for the name that we're going to display would work just as well. (I think I was biased by the testing perspective, the distinguishing feature of the organization that satisfies that test is that it's the only one in our test data that has a SIRET.)

[qbey]

This will break, django-configurations does not expect a list here: https://github.com/jazzband/django-configurations/blob/master/configurations/values.py#L214

[Morendil]

That one is confusing to me, originally I picked up the string value from this file and used it without change in settings.py, but that broke precisely because it was expecting a list of plugin names and not a singular name. (And it seemed reasonable to me that we'd want potentially more than one plugin…) Do we have different semantics between Helm files and settings.py ?

[qbey]

Warning, the indentation has changed here, this might get the wrong "siret".

[Morendil]

I'm not sure I see how to construct a case where we'd get the wrong result here (i.e. a missing unit test). The indent is by intention: I did mean to return directly with the libelle_commune in the commune case (because I'm pretty sure the key is always present) and fall through to the invocation of the private method otherwise. (So we end up with a bit of asymmetry in the code with some error handling in one case and not in the other.)