Support custom attributes on users
What does this MR do?
-
Add an API to get/set custom attributes on users -
Allow filtering users by custom attributes in the API
I plan to add more in follow-up MRs:
- Add an admin UI to manage custom attributes
- Support custom attributes on groups
- Support custom attributes on projects
This contribution is sponsored by Siemens (/cc @bufferoverflow)
Are there points in the code the reviewer needs to double check?
Before proceeding I'd like to get some feedback on the api design. Currently the following routes are added:
GET /:version/users/:id/custom_attributes(.:format) - Get all custom attributes on a user
GET /:version/users/:id/custom_attributes/:key(.:format) - Get a custom attribute on a user
PUT /:version/users/:id/custom_attributes/:key(.:format) - Set a custom attribute on a user
DELETE /:version/users/:id/custom_attributes/:key(.:format) - Delete a custom attribute on a user
Some points of consideration:
- Grape doesn't have built-in support for reusable API modules, I used the approach from
lib/api/time_tracking_endpoints.rb
but saw there's also another (seemingly older) approach inlib/api/subscriptions.rb
- I deliberately kept the
custom_
in the name to avoid confusion with other types of attributes- For filtering I would suggest the same, i.e.
/api/v4/users?custom_attributes[key]=value
(based on @DouweM's suggestion here)
- For filtering I would suggest the same, i.e.
- I only implemented a
PUT
action (instead of bothPOST
/PUT
) since it shouldn't matter to the client if an attribute already exists or not - Not sure if a strictly RESTful approach is sensible here:
- It would be nice if the attributes were returned directly as a hash instead of an array of hashes with
key
/value
keys (i.e.{ foo: "bar" }
vs.[{ key: "foo", value: "bar" }]
) - Clients will probably want to update several attributes at once on each user, so each additional attribute adds a lot of API calls (multiplied by the number of users)
- As an alternative, maybe it would make sense to support mass-assignment of multiple attributes at once? I'm thinking a
POST
action with a hash of key-value pairs (again as{ foo: "bar" }
if we decide to go that route forGET
) - Deleting could also be implemented by passing a
blank?
value
- It would be nice if the attributes were returned directly as a hash instead of an array of hashes with
Let me know your thoughts, I'm also happy with keeping it as it is for now and then optimizing further down the lane ;-)
Why was this MR needed?
Adding custom fields to users is useful for reporting purposes and building other services on top of GitLab.
Screenshots (if relevant)
Does this MR meet the acceptance criteria?
-
Changelog entry added, if necessary -
Documentation created/updated -
API support added - Tests
-
Added for this feature/bug -
All builds are passing
-
-
Conform by the merge request performance guides -
Conform by the style guides -
Branch has no merge conflicts with master
(if it does - rebase it please) -
Squashed related commits together
What are the relevant issue numbers?
Edited by Rémy Coutable