Data loader rework #102
Merged
Data loader rework #102
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This date has deputies change faction during the day so it covers some helpful corner cases
It's not a generic Popolo loader any more so let's not pretend it is.
I don't think there's any reason to call them outside the methods so hopefully this makes the intent clearer.
We need this to look up the party name (what's stored in TVFY) from Popolo group_id (which is what the Popolo data provides).
This allows faction changes in the middle of a day. Fixes #22 It has to fall back to not looking in the `.current_on` member scope because our Popolo data assumes their membership starts the next day. This is all very confusing.
This makes the same change as in 31d77bb to expand the scope of when a member is considered to be in the house. It means that on a day when members change parties within that day they appear to us in both parties. As this ensures all their votes are picked up it seems like a sensible compromise until we can do The Right Thing (whatever that is). It does mean that if a party changes names, some votes can have a whole party absent where their members are actually shown in the other party. This is a bit weird but it's not totally untrue so it's another compromise for the moment.
If you run it with no arguments it will now try to load votes for every day since the most recent one in the database.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This reworks the data loader a bit to accommodate #22.
It also simplifies the rake commands you need to run by baking in the URLs to the data (they can be changed for debugging). It's also a good time to admit that the Popolo loader tasks aren't generic but are specific to Ukraine.
Closes #22.
Example from #22
Before this change
On the first vote they're shown in Група "Партія "Відродження" as absent.
On the last vote they're shown in Група "Партія "Відродження" as absent.
This is clearly wrong.
After this change
On the first vote they're shown in Група "Партія "Відродження" as absent and in Група "Відродження" as not voting (correct).
On the last vote they're shown in Група "Відродження" as absent and in Група "Партія "Відродження" as absent (correct).
It's obviously not right to have the additional party and the extra absent record (extracted into #104)
It's also wrong on their full vote record page. It incorrectly shows him as absent because it's looking up the wrong Member record (extracted into #105).
For the above reason I imagine it's also making the attendance calculation slightly wrong too.
Conclusion
Despite the problems above, it's better than before the change and it's better that what's on the deployed site right now (which is totally wrong!). For this reason I'm going to merge.