-
Notifications
You must be signed in to change notification settings - Fork 19
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me. I don't like our rest api for teams and think it can be improved a lot (#484) but in the context of what we have at the moment, we should merge this and then do a big refactor on the teams api. Thanks @jimmymintzer
lib/plugin/routes/public/teams.js
Outdated
getParams: (request) => ({ teamId: request.params.id }) | ||
} | ||
}, | ||
response: { schema: swagger.List(swagger.NestedTeam).label('PagedTeams') } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will need to be refactored slightly after search is merged and it is rebased
daff2eb
to
5ee1f3d
Compare
added pbac 0.3.0, and updated changes md, updated version nos to pref for release |
|
||
db.query(sqlQuery, function (err, result) { | ||
if (err) return cb(Boom.badImplementation(err)) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested manually with volume data and the results coming back perfect I just noticed if I go outside the range of IDs that it returns an empty array, which is fine but a team that exists could have an empty array so it might give the user the impression that a team exists that doesn't unless we throw back a 404?
@jimmymintzer @dberesford @dberesford just wondering here should we put in 404 'team not found with id' here if the team does not exist... unfortunately 0 results isn't the same thing... maybe a call to get team first returning it's error if it has one, otherwise continue
@cianfoley-nearform Updated to read team first before checking for any nested teams 4a783cf |
looks good @jimmymintzer, could someone else review my checkin I also think we should merge the nested endpoint with standard teams endpoint and list teams in another array along with users in a future revision (see my comment here #484) |
Hi guys, I tested the 404 stuff with volume data and works great... if anyone wants to try out this stuff load up the volume data set npm start pg:init-volume-db and then when making calls using postman or curl or whatever, set the organization to conch using ROOTid... teams 7, 107, 207...507 are parent teams with 99 children between those numbers... e.g. http://localhost:8080/authorization/teams/107/nested shows 99 teams 108/nested shows empty set and 508 returns a 404. If we are happy with everything else I think we can merge now... |
* Split code into 4 packages using lerna. * Reconsiled master merge out to wprl:lerna-ize (#439) * feat: get user teams, no inheritance * fix: get user teams functionality, tests * chore: changes feedback review * chore: release 3.1.0 preps * fix: allow empty results when the user is in no teams * volume data insertion and autocannon bench tests * database migration to 6, added index to team_members, fixed bug in volumeRunner starting server * updated NUM_TEAMs in config to 500 * Forking child process for UDARU server in volumeRunner * tidy ups and upgraded lodash to 4.17.5 * added chalk to volume load/test, using IPC when forking server, updated changes.md * updated intermittently failing user policies test cases * updated pbac version, iam.js also changed as the param order is important and needed to be reversed * support for optional metadata field, team, user, org read only * get, post and put updated for optional metadata field * test cases on core added for create, update, read * #450 solved: authorization pre-check required on teams payload object * version 3.2.0 not 3.2.1 * updates to validatation models for better swagger definitions, enforcement of allow/deny on policies * added test for issue #475 (invalid effect data) * added a note to pagedteamslist to state policies and users not populated * updates to all entities to throw 409 conflict instead of 400 for db insert/update conflict constraints * fix migrate 7 uninstall script * pre-merge issues addressed * fix: add request id for the resource builder * chore: bump package, update changes * update to changes.md summary for 4.0.0 * chore: package lock update * removing public schema from migration scripts * updated version number in package.json and added 4.0.1 entry to changes * support search teams in udaru core * search team endpoint and search logic updates * add total to team search response * update search response validation to match actual response data * Nested Teams Endpoint (#477) * add nested teams to udaru core * add nested endpoint * add e2e test for nested team limit * updating version and changes.md, updated pbac to 0.3.0 * use the buildParams directly to create base resource for user requests * return 404 if nested team is not found * Reset version to 4.0.1, it had accidently been set to 4.2.0 * check if team exists before returning users, check if user exist before return teams * fix typo * Example extended to demonstrate teams (#483) * Support in udaru core for searching users (#463) * Support in udaru core for searching users * Linting fix * Added some sanity check sql injection tests * Linting fix (again) * Linting fix (again again) * expose public user search route * add search users in a team * Updated end to end testing for teams search and team users search * updated version and changes * updated changes.md 4.1.0 date * Migration to nearForm SQL module * Removed postgrator * Removed postgrator * Split code into 4 packages using lerna. * packages/udaru/ -> packages/udaru-core/ * update version in changes.md, packages and static swagger docs
Add support for nested teams by parent team id.
Endpoint:
/authorization/teams/{id}/nested
that takes parameters:headers:
authorization
,org
query:
page
,limit
param:
id
Endpoint necessary for nearform/udaru-ui-admin#3
@dberesford @mihaidma @cianfoley-nearform