Add a wizard to add a new candidacy for a person #968
Conversation
This doesn't look like a big deal, but the next commit will introduce many more imports from forms.
| ) | ||
| with transaction.atomic(): | ||
| person = get_object_or_404(Person, id=kwargs['person_id']) | ||
| LoggedAction.objects.create( |
There was a problem hiding this comment.
Should we not already have the Person from the context?
There was a problem hiding this comment.
Also, I'm a bit confused as why we are looking up the person here and not with the other things above.
There was a problem hiding this comment.
The context is only prepared for GET requests, and done is called when handling a POST request, but you're quite right that it's strange to have two bits of code that looks up the person... I've pushed a fixup that looks it up once by when overriding dispatch.
When you have many elections, the person edit page was taking a huge amount of time to load because it included form fields for every election regardless of whether the person was marked as standing in that election or not. In order to avoid that being required, this commit introduces a wizard for adding a candidacy for a person. This view doesn't require any Javascript, so should be a good basis for progressive enhancement.
With hundreds of elections this resulted in a huge amount of HTML being generated for the candidacy forms, which could crash the browser in trying to render it, and made the generation of the page very slow. This commit removes the code that adds editable candidacy fields for every election to the person edit page. A subsequent commit will replace that with a link to the new "add candidacy" wizard.
Previously, massive numbers of queries were needed to generate the initial data for the person edit form when there were many elections in the database. This commit cuts that down substantially, although there are still a large number of queries needed to generate the page overall.
It is no longer the case that there will be candidacy fields for every election, but having the standing field at its default of required=True would have caused form validation errors if any were missed out. This commit fixes that.
This makes the new "add candidacy" wizard accessible for adding candidacies of other elections to the person edit page.
mhl
force-pushed
the
progressive-add-candidacy-basic
branch
from
485f67c
to
d015057
Compare
| class AddCandidacyPickElectionForm(forms.Form): | ||
|
|
||
| election = forms.ModelChoiceField( | ||
| queryset=Election.objects.order_by('-election_date', 'name')) |
There was a problem hiding this comment.
I think this should filter on current=True too.
There was a problem hiding this comment.
Or is the idea that people should be able to add non-current elections? I think this could be a bit confusing when there are 1000s of election objects in the database.
There was a problem hiding this comment.
@symroe Yeah, my idea was that you need to have some way to add candidacies for historic elections in order to correct mistakes, or complete old data. But you're quite right that it'll be confusing in the more typical case. I've make a pull request that makes that restricts to current=True by default, but links to a version of the picker with all elections: #969
The motivation of this pull request is to deal with the problem that the person edit page included post and party selects for every current election in the database - this might be repeated for hundreds of elections, which makes the page very slow to load and crashes some browsers. (See #740 for more details.)
On the DemocracyClub fork of YNR, this problem has been dealt with by using Javascript to dynamically add those form elements. However, in this upstream versions, so far, all of the site works without Javascript, on the principle that progressive enhancement is a Good Thing, so I was reluctant to merge those changes as-is. This pull request adds a method which is not Javascript-dependent to do the same thing, and we can use this as a base to apply the DC Javascript enhancement in the future. (Which I'm intending to do.)
The person edit page still uses lots of queries in its generation, which is bad, especially since there are fixes for those issues that I made in the DC fork; however, those fixes will apply most easily after the Javascript enhancement has been merged, so my plan is to get this merged, apply the Javascript enhancement of adding new candidacies, and then apply the fixes for excessive queries.
I guess @symroe or @struan might be best placed to review this, if either of you have time? n.b. @symroe, we discussed recently that the formtools wizard examples suggest you need to specify
condition_dictas a keyword parameter toas_view, but it works fine when specified as a class attribute instead, as in this example.