Skip to content

Conversation

begedin
Copy link
Contributor

@begedin begedin commented Oct 17, 2016

As discussed in #341, it's not used at all.

I would like to look at other things here as well. We have tests for filtering by role level and organization id that are written as if they need to come in specific combinations. Migth be simpler to just loosen them up and simply apply supported filters "in order".

@joshsmith
Copy link
Contributor

@begedin so not ready for review yet?

@begedin begedin force-pushed the 341-get-rid-of-organization/memberships_endpoint branch from 6a43308 to 9b2825d Compare October 18, 2016 11:56
@begedin
Copy link
Contributor Author

begedin commented Oct 18, 2016

@joshsmith I removed everything that's unused by ember in a separate commit, so we can roll it back if you don't feel comfortable doing this. However, here's how our org memebrship mirage config looks like:

/**
* Organization memberships
*/

// GET /organization-memberships
this.get('/organization-memberships', { coalesce: true });

// POST /organization-memberships
this.post('/organization-memberships');

// GET /organization-memberships/:id
this.get('/organization-memberships/:id');

// PATCH /organization-memberships/:id
this.patch('/organization-memberships/:id');

// DELETE /organization-memberships/:id
this.delete('/organization-memberships/:id');

We do not have any requirements for filtering, ember side.

When we retrieve an organization, it will provide us a list of membership ids. These ids will then be fetched using coalesced requests.

An average organization will not have members numbering to a point where it would be a problem to get them all and then sort/filter client-side. If we do need additional filters, we can add them later.

If anything, what we should consider is preventing an unfiltered index-request (without a list of ids), or at least paginate or limit that response, or something.

@begedin begedin force-pushed the 341-get-rid-of-organization/memberships_endpoint branch from e33a4e7 to 5eb783e Compare October 18, 2016 12:17
@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 95.263% when pulling 5eb783e on 341-get-rid-of-organization/memberships_endpoint into 593f659 on develop.

@joshsmith joshsmith force-pushed the 341-get-rid-of-organization/memberships_endpoint branch from 5eb783e to f7fd48d Compare October 18, 2016 21:34
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 95.088% when pulling f7fd48d on 341-get-rid-of-organization/memberships_endpoint into b0557dd on develop.

@joshsmith joshsmith merged commit 394933a into develop Oct 18, 2016
@joshsmith joshsmith deleted the 341-get-rid-of-organization/memberships_endpoint branch October 18, 2016 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants