[Feature] Transfer project ownership #4743
Conversation
…oolbox/kpi into feature/transfer-project-ownership
noliveleger
changed the title
…om/kobotoolbox/kpi into feature/transfer-project-ownership
…oolbox/kpi into feature/transfer-project-ownership
…om/kobotoolbox/kpi into feature/transfer-project-ownership
…oolbox/kpi into feature/transfer-project-ownership
noliveleger
changed the base branch from
main
to
refactor-counter-shadow-models
noliveleger
force-pushed
the
refactor-counter-shadow-models
branch
from
d275485 to
752d7eb
Compare
…roject-ownership # Conflicts: # kobo/apps/superuser_stats/tasks.py # kpi/deployment_backends/kc_access/shadow_models.py # kpi/deployment_backends/kobocat_backend.py
noliveleger
changed the base branch from
refactor-counter-shadow-models
to
refactor-api-v2-urls
noliveleger
changed the base branch from
refactor-api-v2-urls
to
lint-format-pre-transfer-project-ownership
…e/transfer-project-ownership
noliveleger
force-pushed
the
lint-format-pre-transfer-project-ownership
branch
from
48ac7a5 to
44f5855
Compare
noliveleger
changed the base branch from
lint-format-pre-transfer-project-ownership
to
main
noliveleger
changed the base branch from
main
to
lint-format-pre-transfer-project-ownership
Conflicts: jsapp/js/project/projectTopTabs.component.tsx
noliveleger
force-pushed
the
feature/transfer-project-ownership
branch
from
1f12a60 to
9e239d7
Compare
| @@ -43,6 +43,9 @@ class InAppMessage(AbstractMarkdownxModel): | |||
| valid_from = models.DateTimeField(default=EPOCH_BEGINNING) | |||
| valid_until = models.DateTimeField(default=EPOCH_BEGINNING) | |||
| last_editor = models.ForeignKey(settings.AUTH_USER_MODEL, on_delete=models.CASCADE) | |||
| # We do not want to use a generic foreign key or tightly couple this model | |||
There was a problem hiding this comment.
If I understand our opinion on generic foreign key, what we dislike is that it points to a django content type which itself is in the database. That adds complexity with no apparent benefit to us. We know the model's name. It never changes. We prefer to reference the model by name instead of a on the dynamic generated ID from content types. Or to say this in a more practical way - gfk's break under certain conditions, such as transferring data from one Django DB to another. There is no guarantee that the content type ID's would line up consistently.
We remain at the risk of it losing the relationship if we did intentionally change the model name/label without a data migration. But in this case, it's fine. Go ahead and loose the connection for some old in app message. Better than a database hitting constraints and erroring out hard.
I'm 100% for this concept. But we do deviate from Django's design. Let's add more detail here (or somewhere?) explaining why we created our own solution to something that comes out of the box with Django. We may end up using it again later.
@jnm I'd be curious on your opinion of this approach.
| value = t(value) | ||
|
|
||
| try: | ||
| transfer = Transfer.objects.get(pk=transfer_id) |
There was a problem hiding this comment.
👍 glad to see it handles a lack of referential integrity, which my suggested design presents us with.
| with transaction.atomic(): | ||
| self._reassign_project_permissions(update_deployment=False) | ||
| self._sent_app_in_messages() | ||
| # FIXME - Draft stay in queue forever |
There was a problem hiding this comment.
Do you mean that you'll fix it in this PR? You marked it as ready for review so I want wondering.
|
It's failing to deploy to staging but looks unrelated. |
Description
Allow users to transfer their project to another user
Notes
The back end support several projects at once per invite but the UI only support one project per invite.
Once a project is being transferred, it sends emails to previous owner and new owner.
it also adds in-app message to all other users who have access to that the project.
In this PR,
deployment__identifier(i.e. link to KoboCAT form representation) to is removed from asset back-end response when project is deployed. It's not 100% related to transfer but the link was based on owner's username. So it should have been updated too. Since the endpoint has been removed in a previous version, the link returns a 404.