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

Improve invite users to organization feature #7138

Merged
merged 89 commits into from
Dec 15, 2023
Merged

Conversation

klakhov
Copy link
Contributor

@klakhov klakhov commented Nov 14, 2023

Motivation and context

This PR improves workflow with invitations to organizations. It allows to view, accept, decline invitations to organizations (previously all the invitations were auto-accepted).
It fixes problems with inviting unregistered users to organization allowing them to register without invitation.

image
image

How has this been tested?

The best way to test this PR is to add those lines to cvat/settings/base.py:
Enable email verification:

ACCOUNT_AUTHENTICATION_METHOD = 'username_email'
ACCOUNT_CONFIRM_EMAIL_ON_GET = True
ACCOUNT_EMAIL_REQUIRED = True
ACCOUNT_EMAIL_VERIFICATION = 'mandatory'

Setup console email backend

# Email backend settings for Django
EMAIL_BACKEND = 'django.core.mail.backends.console.EmailBackend'

And edit ORG_INVITATION_CONFIRM to Yes

ORG_INVITATION_CONFIRM = 'Yes'

Checklist

  • I submit my changes into the develop branch
  • I have created a changelog fragment
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • I have linked related issues (see GitHub docs)
  • I have increased versions of npm packages if it is necessary
    (cvat-canvas,
    cvat-core,
    cvat-data and
    cvat-ui)

License

  • I submit my code changes under the same MIT License that covers the project.
    Feel free to contact the maintainers if that's a concern.

@klakhov klakhov requested review from SpecLad and bsekachev November 20, 2023 09:31
@klakhov klakhov marked this pull request as ready for review November 20, 2023 09:31
@klakhov klakhov requested a review from Marishka17 as a code owner November 20, 2023 09:31
cvat-ui/src/actions/invitations-actions.ts Outdated Show resolved Hide resolved
cvat-ui/src/actions/invitations-actions.ts Outdated Show resolved Hide resolved
cvat-ui/src/components/invitations-page/empty-list.tsx Outdated Show resolved Hide resolved
cvat-ui/src/components/invitations-page/styles.scss Outdated Show resolved Hide resolved
@@ -300,7 +309,7 @@ class CVATApplication extends React.PureComponent<CVATAppProps & RouteComponentP
loadAuthActions();
}

if (user == null || !user.isVerified) {
if (user == null || !user.isVerified || !user.id) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can we have a user without id?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When user is registered (if email verification is disabled), we get user data from response of /register, and /self request is not sent. There is no id in such data

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably we should fix /register endpoint instead

Copy link
Member

@bsekachev bsekachev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Client part LGTM
Please, fix eslint issues:

/home/runner/work/cvat/cvat/cvat-ui/src/components/invitations-page/invitations-list.tsx
Error: 5:30 error 'useEffect' is defined but never used @typescript-eslint/no-unused-vars
Error: 13:8 error 'notification' is defined but never used @typescript-eslint/no-unused-vars

cvat/apps/iam/rules/organizations.rego Show resolved Hide resolved
cvat/schema.yml Show resolved Hide resolved
cvat/apps/iam/serializers.py Outdated Show resolved Hide resolved
@SpecLad
Copy link
Contributor

SpecLad commented Dec 13, 2023

Changed reject to decline.

I can still see reject everywhere in the backend code. It should be updated too.

@klakhov
Copy link
Contributor Author

klakhov commented Dec 15, 2023

It looks we have a couple of unstable e2e tests right now:
actions_users/registration_involved/case_28_review_pipeline_feature. js
actions_projects_models/registration_involved/base_actions_project_task_user.js

They failed in this pr a couple of times(after rerun everything is ok)
And they are failing in other PRs.

I will merge this PR as its already approved, and invistigate test problems separately.

@klakhov klakhov merged commit 2b7d01a into develop Dec 15, 2023
32 of 33 checks passed
@bsekachev bsekachev deleted the kl/improve-invite-users branch December 20, 2023 09:26
@cvat-bot cvat-bot bot mentioned this pull request Jan 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants