Conversation
chris48s
force-pushed
the
sopn-name1-20260108
branch
from
24e8667 to
6af70c9
Compare
This comment was marked as outdated.
This comment was marked as outdated.
chris48s
force-pushed
the
sopn-name1-20260108
branch
from
49827a3 to
f1e559c
Compare
| "sopn_last_name": candidacy.get("sopn_last_name", ""), | ||
| "sopn_first_names": candidacy.get("sopn_first_names", ""), |
There was a problem hiding this comment.
This is the biggest question I have on this PR.
I was a bit unsure about using .get("key", "") vs ["key"] in this PR.
I decided to use .get("key", "") here because if we have parsed any SOPN before this is deployed then we're not going to have those keys and we'll throw a KeyError
but then later on in the flow, I've switched to ["key"] but really we have the same issue. If we created a RawPeople object before the deploy, then the code we run after will throw a KeyError.
I wonder if it would make sense to either:
- Write a data migration to go through any existing parsed SOPN data and add
"sopn_last_name": "", "sopn_first_names": ""to every candidate. Then we can just always assume those keys exist - Write a data migration to just nuke any parsed SOPN data that exists at the point we deploy on the basis we can re-parse the PDFs. Again, then we can just always assume those keys exist
Thoughts or guidance on this would would be really useful
There was a problem hiding this comment.
I expect the right thing to do here is to model the JSON using some sort of data class (native or Pydantic, etc) and validate against that at run time.
We're not going to do that right now though, are we?
I forget, do we delete the raw people objects after we've added the data in the bulk adding forms? In that case, there aren't going to be many at all outside of a major electon. I think I'm ok with deleting them.
If we never delete any of them, then I reckon the migration is a better idea than having to re-parse everything.
There was a problem hiding this comment.
Yeah good thoughts. We seem to keep the AWSTextractParsedSOPN objects around forever, so I guess it makes sense to tolerate the key being missing there. We clear up the RawPeople object after the bulk add process. I had a look at how many RawPeople objects there are in the prod database right now. There's just shy of 100 in there. The most recent was created in June 2025. I suspect they're mostly ones that should have been cleared up but never were. I think that means that just deleting any existing RawPeople objects is the way to go. I'll proceed with that.
| @admin.display(description="Membership object") | ||
| def admin_link(self, obj): | ||
| if not obj.pk: | ||
| return "-" | ||
| url = reverse( | ||
| "admin:popolo_membership_change", | ||
| args=[obj.pk], | ||
| ) | ||
| return format_html('<a href="{}">Open</a>', url) | ||
|
|
There was a problem hiding this comment.
This change isn't really strictly related to this PR, but I did find myself wanting it while I was working on this, so I've quickly pushed it onto this branch.
|
build failures are due to GitHub outage |
chris48s
force-pushed
the
sopn-name1-20260108
branch
from
723fb0e to
cc6ae6f
Compare
2 participants
The scope of this PR is just to do the behind the scenes bits.
So in this PR I have:
This is a chunk of work that is worth reviewing and deploying, but it is not 100% of what we want to do here.
The second bit of work here will be to allow the user to modify these fields through the frontend and provide some way to discover profiles which are missing this data
I'll do those in a follow up PR (or maybe 2 follow up PRs)