Closed mhl closed 7 years ago
This is great! Thanks Mark.
In future we might want to disable adding candidates to non-current elections, for example, for May 2016 in the UK we know we had 100% of candidates, so adding a new one would always be wrong.
I think this would be easy to add as a setting like ALLOW_ADDING_HISTORIC_CANDIDACIES
, and then looking for that in this form.
EDIT: once again I'm too slow in my review!
You're not too slow, it's still open and unmerged!
Oh yes – I looked saw the merged tag on the link to the other PR and thought is was this one :)
In future we might want to disable adding candidates to non-current elections, for example, for May 2016 in the UK we know we had 100% of candidates, so adding a new one would always be wrong.
I think this would be easy to add as a setting like ALLOW_ADDING_HISTORIC_CANDIDACIES, and then looking for that in this form.
I'm not sure about this, but it feels to me that this is already covered by being able to lock posts, which already stops new candidacies being added (by unprivileged users). To take the May 2016 example, all the posts in those elections are locked now, I think, so only people in the "Trusted to Lock" group would be able to add candidacies via this means anyway. Do you think an ALLOW_ADDING_HISTORIC_CANDIDACIES
setting would be useful in addition to the locking?
Oh that makes sense. Maybe it's fine then, although it might result in an odd error if someone tries to add someone to an election with a locked post.
I expect it's such a rare case that it doesn't actually matter.
The error you get on selecting an election with only locked posts is OK (if a bit ugly):
... but yeah, if there are some unlocked posts in the election you just get a 500 at the final step of of the wizard. I think I'll create a new issue for that, though, since, as you say, it's a pretty rare case, and that was a bug already present before this PR.
Excellent :) In that case, this PR looks good to me :)
Thanks for the review, Sym - I've merged this and the new issue is https://github.com/mysociety/yournextrepresentative/issues/970
Previously the default was to show all elections, whether they were current or not, but @symroe pointed out that there might be thousands of such elections, which in normal use would be irrelevant.
This commit makes the inclusion of non-current ('historic') elections in the election picker stage an option - they're only shown if there's an 'historic=1' query parameter. There's also a link to that version of the picker from the normal one.