mysociety / popit-api

DEPRECATED - Development on PopIt has stopped and it is no longer being maintained
https://goo.gl/Vvej4Q
Other
17 stars 3 forks source link

Create/update memberships included inline in documents #102

Closed struan closed 9 years ago

struan commented 9 years ago

This changes the API to process memberships included in POSTs and PUTs which create or update persons, organizations or posts. If a membership key is included in these then the memberships of the document will be updated to match this. It will create, update and delete memberships as required in order to do this.

If no memberships key is present in the document them memberships will be unchanged.

chrismytton commented 9 years ago

Overall this looks great! There are a few instances where there are unnecessary callback functions where you could simply pass done/next in directly, as I described above, which would cut down the line count a bit.

My main concern is that this adds a lot of complexity to src/app.js, would it be possible to pull out removeMemberships, restoreMemberships, tidyUpInlineMembershipError, checkMembership, createMembership, processMembership and removeOldMemberships into a separate module (e.g. src/inline-memberships.js) and then require that from app.js?

chrismytton commented 9 years ago

Just discussed this with @struan and we've decided to hold off merging this until we've bumped the API version - https://github.com/mysociety/popit/issues/687.

struan commented 9 years ago

Superseded by https://github.com/mysociety/popit-api/pull/112 which is in a new PR as the merge was horrible and doing it didn't result in any benefits.