Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow organizations to add identical posts via num_seats #167

Closed
wants to merge 6 commits into from
Closed

Allow organizations to add identical posts via num_seats #167

wants to merge 6 commits into from

Conversation

mileswwatkins
Copy link
Contributor

No description provided.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 95.13% when pulling a5e7890 on mileswwatkins:miles/feature/post_num_seats into 80b6565 on opencivicdata:master.

@jpmckinney
Copy link
Member

When it comes to assigning people to posts through memberships, won't Pupa currently assign everyone to the same post among the identical posts?

@mileswwatkins
Copy link
Contributor Author

Correct, @jpmckinney. I misunderstood my conversation with @paultag. Also, sorry for the messy PR; I hadn't gotten the tests to run until yesterday evening.

All tests running green locally together, for both python-opencivicdata-django and pupa repos.

@mileswwatkins
Copy link
Contributor Author

@jpmckinney
Copy link
Member

Paul, James and I had a discussion about num_seats over a year ago; I had thought the conclusion was to instead create multiple posts. The discussion was recently revived on the Poplus list, where I summarize the pros/cons (in my second chunk of text) of having multiple posts versus having some "meta-post" with a property like num_seats; the multiple posts option still comes out on top in that discussion. I recommend joining the Poplus list as there are often relevant discussions about Popolo.

@mileswwatkins
Copy link
Contributor Author

Thanks for the links, James! Paul and I had a chat in-person, and he's going to chime in here when he gets a chance.

@paultag
Copy link
Contributor

paultag commented May 19, 2015

I think we'll likely go with num_seats, I don't think it's semantically wrong (but I don't remember that thread) - and this will make things like matching to the same post (in the DB, the same object) much easier to do right now.

Multiple posts seem fine too, but I don't see any big upside, but it has a lot more complexity in the import machinery, was there a downside to num_seats?

@jpmckinney
Copy link
Member

In the Canadian scrapers, we handle this by tracking seat numbers which is not crazy complicated, but also not super clean.

The semantics are wrong, because the semantics of Post is that it is only held by one person at a time. What you're describing is a PostSet (not in Popolo at the moment), where the default maximum cardinality is 1, but it can be set to some other number.

num_seats is undesirable because (1) we avoid abbreviations, (2) Posts are not always seats, (3) it's ambiguous as to whether it's the number of currently held or possibly held Posts.

I would name the property maximum to accurately indicate the maximum number of Memberships that can exist concurrently relating to this Post, and ideally rename the class Post to PostSet, though I guess that may be a bridge too far.

@mileswwatkins
Copy link
Contributor Author

Closing this for now, may re-open in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants