Duke-GCB / D4S2

Web service to facilitate notification and transfer of projects in DukeDS
MIT License
0 stars 0 forks source link

Allow fetching duke-ds-project permissions #170

Closed johnbradley closed 6 years ago

johnbradley commented 6 years ago

Changes add query parameter is_deliverable to the /api/v2/duke-ds-projects/ endpoint. Also returns is_deliverable in the payload for duke-ds-project.

Fixes #169

johnbradley commented 6 years ago

I am having second thoughts about these changes in relation to performance. It takes 11 seconds to query the list for my user with 63 projects. If I skip fetching project permissions it drops to 2.7 seconds.

dleehr commented 6 years ago

Do you think we might want to add caching in some form?

There might be a simple Django or DRF plugin that solves this handily for these DukeDS API responses that don’t change every second. And there’s always redis or memcached. Probably a good idea for user listing too.

johnbradley commented 6 years ago

Do you think we might want to add caching in some form?

I worry about timely cache invalidation if we cache project permissions. First time users will wait ~11 seconds while we populate the cache.

One thought was to turn this around and check permissions at the point the try to proceed with a delivery. A user sees all projects and can select them and then we show them a warning like You must have admin priviledges to deliver this project. This feels like we are just moving user frustration further down the road.

I looked to see what the DukeDS portal does: It fetches permissions for each project individually.

Perhaps I could look at paging the data?

dleehr commented 6 years ago

One thought was to turn this around and check permissions at the point the try to proceed with a delivery.

One thing that occurs to me on reflection is that neither method is a great experience for the user.

If we filter out the non-deliverable ones, users might be confused about why they're seeing more projects listed in the DDS web portal vs the datadelivery UI.

But if we include all of their projects in the listing, we have to communicate which ones cannot be delivered and why. If we don't deliver this feedback until late in the process, that can be frustrating.

So perhaps the best tradeoff for now is to allow them to pick any of their projects, then check the permission and deliver feedback before proceeding to the next step?

johnbradley commented 6 years ago

I removed theis_delivery property/filtering from the delivery endpoint. I added a new /api/v2/duke-ds-project-permissions/ endpoint to allow checking project permissions.

The underlying DukeDS API requires a project_id: https://api.dataservice.duke.edu/apiexplorer#!/projects/getApiV1ProjectsProjectIdPermissions To work around this the endpoint requires project_id query parameter to return a list of permissions. There is also an optional user_id filter.

The underlying DukeDS API doesn't return a unique ID. To work around this the code generates one by combining the project and user ids returned.