Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/project ownership #346

Merged
merged 43 commits into from
Mar 28, 2022
Merged

Feature/project ownership #346

merged 43 commits into from
Mar 28, 2022

Conversation

teemukataja
Copy link
Contributor

@teemukataja teemukataja commented Feb 7, 2022

Description

Folders and templates were originally owned by users, now they will be owned by projects that users belong to, allowing multiple users to work on the same folders and templates at the same time.

Related issues

Fixes #282
Frontend issue CSCfi/metadata-submitter-frontend#625

Type of change

  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update (API changes)

Changes Made

  • deprecated folders and templates keys from user
  • added new key projects to user
  • added new key projectId to folder and template collections
  • added new project collection
  • new mandatory userinfo value from AAI sdSubmitProjects
  • if user has no sdSubmitProjects they are redirected to /noproject
  • new mandatory query parameter projectId for GET /folders
  • new endpoint GET /templates?projectId=... that replaces GET /users/current "templates":[...]
  • projectId is not mandatory when requesting specific resource, e.g. GET /folders/{folderId}, the permissions are read from session
  • new mandatory key projectId for POST /folders and /templates
  • deprecated PATCH /user
  • new request body keys index and tags in PATCH /templates/schema/templateId which are the same values as previously used in the deprecated PATCH /user

Other Points

  • user project affiliations are re-validated on every login, if project affiliations have changed, user data is updated
  • breaking change that requires fresh database, because "project" is new information that did not exist before, and it can't be migrated to existing user-owned hierarchy due to missing relationship information

Testing

  • Unit Tests
  • Integration Tests
  • Manual Testing

@teemukataja teemukataja marked this pull request as draft February 7, 2022 08:19
genie9 and others added 21 commits February 14, 2022 08:59
Filename is needed for db entry (fileName and displayTitle)
when new object created from file and is assigned to folder.
Moving responsibility for adding newly created object to a folder from frontend to object endpoint.
Now folder id is required query parameter with POST object/.
Tests updated.
Update spelling wordlist.
Bumps [pytest](https://github.com/pytest-dev/pytest) from 7.0.0 to 7.0.1.
- [Release notes](https://github.com/pytest-dev/pytest/releases)
- [Changelog](https://github.com/pytest-dev/pytest/blob/main/CHANGELOG.rst)
- [Commits](pytest-dev/pytest@7.0.0...7.0.1)

---
updated-dependencies:
- dependency-name: pytest
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [pip-tools](https://github.com/jazzband/pip-tools) from 6.5.0 to 6.5.1.
- [Release notes](https://github.com/jazzband/pip-tools/releases)
- [Changelog](https://github.com/jazzband/pip-tools/blob/master/CHANGELOG.md)
- [Commits](jazzband/pip-tools@6.5.0...6.5.1)

---
updated-dependencies:
- dependency-name: pip-tools
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
@blankdots blankdots linked an issue Feb 15, 2022 that may be closed by this pull request
@teemukataja
Copy link
Contributor Author

teemukataja commented Feb 16, 2022

The previous commit 00cec8f

  1. updated integration tests,
  2. they also showed some bugs in the code which were fixed in the same commit
  3. also removed some deprecated features, and will continue to prune through deprecated features in next commit

@teemukataja
Copy link
Contributor Author

Added integration test case for GET /templates. I tried to copy folder unit tests (query_folders) for templates (query_templates_by_project) (because they are almost identical), but because the query loops over collections, it raises a StopAsyncIteration, and I'm not sure how to deal with that.

Would this be usful:

try:
    ....    
except StopIteration:
    pass

I thought about this also initially and tested it, but it still errored, and I think it's also not the correct way to approach this, because it would stop the test

This is the function
https://github.com/CSCfi/metadata-submitter/blob/feature/project-ownership/metadata_backend/api/operators.py#L308-L365
It queries multiple collections

It's based on this
https://github.com/CSCfi/metadata-submitter/blob/feature/project-ownership/metadata_backend/api/operators.py#L812-L854

@blankdots
Copy link
Contributor

wait you want query_templates_by_project not to filter the templates content ... 🤔 maybe it will be easier if you put them all in one collection templates and add the type for each document in the collect e.g. study, sample etc. I am not sure we should invest the effort in making cross collection queries at this point for templates

@blankdots
Copy link
Contributor

however if you really want JOIN like from SQL i think https://docs.mongodb.com/manual/reference/operator/aggregation/lookup/ is what you need.

@teemukataja
Copy link
Contributor Author

teemukataja commented Feb 23, 2022

It's based on this https://github.com/CSCfi/metadata-submitter/blob/feature/project-ownership/metadata_backend/api/operators.py#L812-L854

I think this: https://github.com/CSCfi/metadata-submitter/blob/feature/project-ownership/metadata_backend/api/operators.py#L395 might be more appropriate as you want object query not folder query and maybe pay attention to: https://github.com/CSCfi/metadata-submitter/blob/feature/project-ownership/metadata_backend/conf/conf.py#L113

That is also the same, it uses self.db_service.do_aggregate() which is all this function needs

For the query constructor, I don't think that is needed for this use case (seems overly complicated), because we only want to read 2 values (accessionId and title)

wait you want query_templates_by_project not to filter the templates content ... 🤔 maybe it will be easier if you put them all in one collection templates and add the type for each document in the collect e.g. study, sample etc. I am not sure we should invest the effort in making cross collection queries at this point for templates

I didn't want to create new functionality, but re-use existing data to keep it simple. This is an endpoint that replaces GET /users/current's "templates": [...] key

however if you really want JOIN like from SQL i think https://docs.mongodb.com/manual/reference/operator/aggregation/lookup/ is what you need.

I will look into this if it makes it possible to request the data in one query

tests/test_handlers.py Outdated Show resolved Hide resolved
@teemukataja
Copy link
Contributor Author

however if you really want JOIN like from SQL i think https://docs.mongodb.com/manual/reference/operator/aggregation/lookup/ is what you need.

I will look into this if it makes it possible to request the data in one query

Looks like lookup is used for joining data together into one object from separate collections, but we want to get a list of all objects from multiple collections that have the same project id

@teemukataja
Copy link
Contributor Author

teemukataja commented Feb 28, 2022

Returned old object assigning code into use with templates-in-project (as opposed to templates-in-user).

@blankdots blankdots added this to the 0.12.0 milestone Feb 28, 2022
@juhtornr juhtornr modified the milestones: 0.12.0, 0.13.0 Mar 14, 2022
@teemukataja teemukataja merged commit 59de0ba into develop Mar 28, 2022
@teemukataja teemukataja deleted the feature/project-ownership branch March 28, 2022 07:27
@blankdots blankdots mentioned this pull request Apr 7, 2022
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Folders can be owned by multiple users
4 participants