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

feat(frontend): various improvements around invitations #2993

Merged
merged 5 commits into from
Oct 7, 2022

Conversation

usu
Copy link
Member

@usu usu commented Sep 24, 2022

Various improvements from this epic: #1238

More specifically:

  • Bei Einladung "Login" -> redirect zurück zu Einladung (OK) ; Daten werden aber nicht neu geladen -> Einladung wird nicht korrekt angezeigt
  • store module to have a reactive user property to check if a user is logged in (and if so which one)
  • --> this allowed to remove NavigationAuth and replace by NavigationDefault. Didn't verify/fix the mobile view, as this would interfere a lot with Make new navigation prototype #2576
  • fix error when resending an invitation to a CampCollaboration, where invitee is an existing user and not just an unknown email address

Open questions

Invitation endpoint, property userDisplayName

Should be: "The display name of the user that is invited. May be null in case the user does not already have an account."
By mistake: returns DisplayName of the currently authenticated user
This was used in Invitation.vue to display the currently logged in user.
Do we need/want this property at all or shall we remove from the endpoint?

Decision: Can be removed

Invitation endpoint, property userAlreadyInCamp

Indicates whether the logged in user is already collaborating in the camp, and can therefore not accept the invitation.
This is also true if a CampCollaboration exists but is set to inactive.
User receives a message "You are already participating in this camp" with an "Open camp" link. Open the camp doesn't work though, because the user is inactive.
What should be the intended user experience? We could provide a message like "You're set to inactive. Ask a camp manager to set your status to active" or similar. However, this is a bit odd because the user apparently has a valid inviteKey and could theoretically accept the invitation as another user, but not with this one.

Decision:

  • Invitations on existing users belong to this user --> cannot be taken over
  • Invitations on email addresses --> on accepting with an inactive user, the responsibility is merged with the existing one

@usu usu mentioned this pull request Sep 24, 2022
18 tasks
@usu usu marked this pull request as ready for review September 24, 2022 16:38
Copy link
Contributor

@BacLuc BacLuc left a comment

Choose a reason for hiding this comment

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

Thank you.

<auth-container v-if="ready">
<div v-if="invitationFound">
<h1 v-if="userDisplayName" class="display-1">
{{ $tc('components.invitation.userWelcome') }} {{ userDisplayName }}
Copy link
Contributor

Choose a reason for hiding this comment

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

This was here on purpose to welcome the user by his name (if logged in).
Before this was the possibility used to check if the user was logged in or not.

Also from the background that the recipient of an invitation may use another account to accept the invitation.

Now you see your name in the navigation, so we can remove userDisplayName if we want to use this way.

@@ -18,7 +18,7 @@
block
@click="acceptInvitation"
>
{{ $tc('components.invitation.acceptCurrentAuth') }}
{{ $tc('components.invitation.acceptCurrentAuth') }}<br />
Copy link
Contributor

Choose a reason for hiding this comment

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

I would remove the
here.

@@ -8,9 +8,28 @@ Vue.use(storeLoader)
// expired on 01-01-1970
const expiredJWTPayload =
'eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiJ9.eyJpYXQiOjE2MzMxMzM0MDksImV4cCI6MCwicm9sZXMiOlsiUk9MRV9VU0VSIl0sInVzZXJuYW1lIjoidGVzdC11c2VyIiwidXNlciI6Ii91c2Vycy8xYTJiM2M0ZCJ9'
// {
Copy link
Contributor

Choose a reason for hiding this comment

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

minor:
i would use a multi line comment for multi line texts.
This makes copy/paste easier.

@usu usu added the Meeting Discuss Am nächsten Core-Meeting besprechen label Oct 1, 2022
@BacLuc
Copy link
Contributor

BacLuc commented Oct 1, 2022

Invitation endpoint, property userAlreadyInCamp

Indicates whether the logged in user is already collaborating in the camp, and can therefore not accept the invitation.
This is also true if a CampCollaboration exists but is set to inactive.
User receives a message "You are already participating in this camp" with an "Open camp" link. Open the camp doesn't work >though, because the user is inactive.
What should be the intended user experience? We could provide a message like "You're set to inactive. Ask a camp manager to >set your status to active" or similar. However, this is a bit odd because the user apparently has a valid inviteKey and could >theoretically accept the invitation as another user, but not with this one.

This happens if you set the user to inactive after inviting the user?

I would say the invite link of an inactive user should not exist anymore.
-> if we set the user to inactive, we remove the inviteKeyHash.

Another way would be to check the status, but then we still have the inconsistent state in the db.

@usu
Copy link
Member Author

usu commented Oct 1, 2022

This happens if you set the user to inactive after inviting the user?

No not exactly. I tested the following use case:

  1. I have a valid user account on ecamp ([email protected])
  2. I was member of a camp X, but then someone deactivates my collaboration in the camp (status inactive)
  3. I recognize that I don't have access to to the camp anymore, so I ask someone to send me an invitation
  4. Someone sends me an invitation to [email protected] (non-existing user in ecamp), instead of reactivating my existing collaboration
  5. I grab the invitation link & login with my existing user ([email protected])
  6. eCamp tells me, that I'm already part of the camp (userAlreadyInCamp: true). I cannot accept the invitation, but I can also not open the camp.

@usu usu removed the Meeting Discuss Am nächsten Core-Meeting besprechen label Oct 7, 2022
@usu usu enabled auto-merge October 7, 2022 23:36
@usu usu merged commit 35a3889 into ecamp:devel Oct 7, 2022
@usu usu deleted the feat/invitation-workflow branch November 6, 2022 05:54
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.

3 participants