Question assignment

[dantheta]

Script to populate the answers table for candidates. Also introduces the participating flag on candidate, which could be set during their signup process. Could look at refactoring this so that it can be run on a candidate immediately after they've signed up.

[symroe]

Couple of comments on this:

1. Can this be a management command, not a standalone script?
2. Why are you using raw SQL in the middle of the script, and not the ORM? The query isn't hard, so it should really use the django ORM
3. If you need to populate answers (I don't understand the reason for this, but I've not read the related conversations), could it be done as part of the initial save of a question? Maybe it's because candidates are added over time? If that's the case I think the answers app should define a post_save hook that looks for the initial save (creation) of questions or candidates.
4. Other comments inline

[symroe]

Also, looks like the migrations are missing from this commit. Can you force push again with them included in the same commit?

[symroe]

One more thing! Can the script have some tests please? :)

[dantheta]

I should probably have prefaced this with "I don't know very much django at all".

The empty answers are added in their own step because new questions and candidates are being added over time. We don't want late-arriving candidates to miss the early questions, and we don't want questions to go unanswered just because they didn't exist when the candidates are active. It seemed fairly straightforward to maintain a fixed-size set of open questions against each candidate and fill that set with unanswered questions when candidates arrive or as questions are added. As I mentioned in the earlier message, this could be rearranged so that the webapp can set up open questions for new candidates at an arbitrary point in time.

The ORM one was difficult - I new the exact SQL that I was after, but a considerable amount of googling and manual reading didn't yield anything that worked. I couldn't get the ORM to add the candidate clause to the join condition. If you've got an example I can look at, that'd be great.

The migrations should be OK too. Tests might take a little longer. How do you want database tests to work? Is it OK for them to write to an actual database, or do you want mock objects?

[symroe]

On testing: the django test runner will make a test database for you. It will use SQLlite for this if you add it to your settings (it shoud be in testing.py really): https://github.com/DemocracyClub/candidate_questions/blob/master/q_and_a/settings/local.py#L22

This means you can write tests without worrying about the main database. You should read up on testing in django: https://docs.djangoproject.com/en/1.7/topics/testing/

I think the ORM query you want is:

Question.objects.filter(answer=None, answer__candidate=candidate.popit_id)

But I could be wrong there. Either way, you want __ to span queries.

From what I understand of what you're doing I think you should make this in to a signal rather than a management command as previously suggested.

https://docs.djangoproject.com/en/1.7/ref/signals/#post-save

[dantheta]

The query-spanning bit I picked up quite quickly - it was the placement of the candidate_id that was the tricky part. The ORM example that you've given puts the candidate_id condition in the where clause, which doesn't do the same thing, unfortunately.

I'll take a look at the post-save hook. I'd like to keep the management command as an option - it would be useful for populating the system after an import of questions or candidates (or if we find that running the routine on every question/candidate save is too expensive).

[symroe]

Ok, I obviously didn't parse the query properly, but I think you can do it easily. Maybe with https://docs.djangoproject.com/en/1.7/ref/models/querysets/#prefetch-related?

I know the SQL is easier, but it makes the app less portable, and it's normally better to try to use the ORM. I'm personally much happier with SQL, but I think it's important to comply to standard patterns within a framework.

On the management command, maybe move this to a method on Organisation and have it called from a signal and a command?

[dantheta]

I run into this sort of thing a lot with ORMs, it seems. I'm certainly happy to use them for the easy stuff. Although this sort of query looks straightforward, it's getting involved with the join criteria which does seem to be the point at which the ORM starts to fight back.

I can rewrite it to play nicely with the ORM - it just winds up being more expensive.

[dantheta]

I'm going to have to give up, I'm afraid. The migrations stuff is a terrible mess, and I can't get the ORM to do anything that is anywhere near as good as that SQL query. I'm going to redo this branch as best I can (hopefully getting the migrations right this time), and submit another pull request based on that.