[WIP] #2341 Allow re-generation of anyone-can-join invite link #2351

SebinSong · 2024-09-16T05:55:00Z

[NOTE 1] As in the screenshot below, if the invite's life is > year, the human-readable expression says 'X yrs Y mth Z d' instead of displaying the hours and minutes too. But let me know if this should be updated otherwise.

[NOTE 2] If it's the case where the group-invite should be generated straight away, the button displays 'Create invitation' instead of 'Send invitation'. But let me know if this should be reverted.

…yone-can-join-invite-issue

SebinSong · 2024-09-16T05:57:49Z

frontend/views/containers/dashboard/GroupMembers.vue

-    action: {
-      type: String,
-      default: 'addMember',
-      validator: (value) => ['addMember'].includes(value)


as the validator here shows, only available value for this prop is 'addMember' which is also set as the default. So just removed it to clean it up.

SebinSong · 2024-09-16T05:58:43Z

frontend/views/containers/group-settings/InvitationsTable.vue

-      const MIL = 1000
-      const MIL_MIN = 60 * MIL
-      const MIL_HR = MIL_MIN * 60
-      const MIL_DAY = 24 * MIL_HR


These constants are already in time.js of the project. so replaced these with them.

cypress · 2024-09-16T06:06:04Z

group-income Run #3169

Run Properties: Passed #3169 • b746711df7 ℹ️: Merge b31dd3e6fa4e1254cabc056e87148ba4639a26c1 into 9fd5e1643cf0396ad0dfb58b6d45...

Project	`group-income`
Branch Review	`sebin/task/#2341-anyone-can-join-invite-issue`
Run status	`Passed #3169`
Run duration	`09m 15s`
Commit	`b746711df7 ℹ️: Merge b31dd3e6fa4e1254cabc056e87148ba4639a26c1 into 9fd5e1643cf0396ad0dfb58b6d45...`
Committer	`Sebin Song`
View all properties for this run ↗︎

Test results
Failures	`0`
Flaky	`0`
Pending	`10`
Skipped	`0`
Passing	`111`
View all changes introduced in this branch ↗︎

…yone-can-join-invite-issue

taoeffect

Nice job @SebinSong, this is a good start. 👍

I think to really nail this it would be useful for me to send you the data folder from the debug instance we have so that you can test this locally with a group that had its invite expired. See your DMs on GIG!

Currently, you'll notice that when you login as u1, the Turtles (4) has one member and when running with this build, clicking + Add to add members will bring up this dialog:

There are two issues here:

The invite link that is shown is expired. We need to modify this PR to detect when these original anyone-can-join links are expired and replace them with new ones. I will comment down below with the logic for this in a separate section.
The expiry date doesn't show the year here, it should though because now we have invite links expiring 20 years into the future.

Logic on how to upgrade expired anyone-can-join links

What we want to do is detect when the group has an anyone-can-join link that meets certain criteria for replacement. These can be arbitrary, but for now, let's make it simple:

If the anyone-can-join link is expired, and it has invites remaining, then we will create another anyone-can-join link in its place that has 150 - (previous used invites count) invites remaining (EDIT: I changed this from 60 to 150)

Then there is the question of where to do this?

We want to make sure that when the user goes to invite someone, that they have this new link available to them.

So where is "when the user goes to invite someone" triggered?

It's triggered in 2 places as far as I can tell:

When the user clicks the "+ Add" button
When the user switchings to the Group Settings page (e.g. the mounted () call there)

So we need to be prepared and already have the new link ready for them in those two places, and we need to keep the code DRY.

So I propose creating an action in gi.app namespace that gets called from both those places. E.g. gi.app/fixAnyoneCanJoinLink or something like that.

You should be able to call it multiple times safely (e.g. in case when they first call it they're offline, and then they need to call it again).

Hope that makes sense, let me know if you have questions!

SebinSong · 2024-09-20T21:00:58Z

@taoeffect Thanks, received the data for debugging in GIG. will update this PR once done with investigating it.

corrideat · 2024-09-22T17:53:42Z

frontend/model/contracts/shared/constants.js

@@ -92,7 +92,7 @@ export const MESSAGE_TYPES = {
 }

 export const INVITE_EXPIRES_IN_DAYS = {
-  ON_BOARDING: 30,
+  ON_BOARDING: 365 * 20, // 20years - we shouldn't let the anyone-can-join invite expire. (reference: https://github.com/okTurtles/group-income/issues/2341)


20 years is a long time, but if the intention is for the invite not to expire at all, why not just have it not expire?

@corrideat how do we do that? I don't remember how the invite stuff you implemented works. Does it support non-expiring invites? If so, how?

We support that, though not in the UI apparently.

In actions/group.js, if you skip the 'expires', the invite will not have an expiry time (although the modal will show 'This invite link expires on Invalid Date.' and the group settings will show it as active but 'expired').

/* expires: Math.min( Date.now() + 1 * 60 * 1000, Date.now() + DAYS_MILLIS * INVITE_EXPIRES_IN_DAYS.ON_BOARDING ), // TODO: revert this after development */

We support this 'feature' already for the group creator to join the group (who joins using the CSK instead of an invite link). This way, the invite link doesn't show as 'used' which would be confusing.

…yone-can-join-invite-issue

SebinSong · 2024-09-23T22:29:38Z

@taoeffect
I just investigated with the debug data you shared and clicking on + add button actually spawns this AddMember proposal modal instead. (when trying it on master FYI)

But the solution you proposed which is,

If the anyone-can-join link is expired, and it has invites remaining, then we will create another anyone-can-join link in its place that has 150 - (previous used invites count) invites remaining (EDIT: I changed this from 60 to 150)

Still need to be implemented in Group Settings.
So let me work on adding this logic in Group settings page as an additional update.

SebinSong · 2024-09-24T07:18:22Z

@taoeffect I've worked on your last change request which is creating gi.app/group/fixAnyoneCanJoinLink action in this commit.

To summarize what it does,
1. Created gi.contracts/group/updateGroupInviteExpiry which allows us to update the expires attribute of a group's invite (here).
2. gi.app/group/fixAnyoneCanJoinLink will call thisgi.contracts/group/updateGroupInviteExpiry to update the anyone-can-join invite to have 20 years as its expires.

(let me know if you have better idea for above though.)

This seems to work well (auto-updated to 20yrs)

except when I use this invite in another browser/container, it still says it's expired.

And apparently it's because the group state loaded by sbp('chelonia/latestContractState') in Join.vue still has the old state before 1. above.

group-income/frontend/views/pages/Join.vue

Line 125 in 9fd5e16

const state = await sbp('chelonia/latestContractState', groupId)

Q. What do you think is the missing piece in this flow here which makes sure sbp('chelonia/latestContractState') loads the group state with the latest update made in 1.?

Hope my explanation here makes sense btw.
Cheers,

taoeffect · 2024-09-24T18:32:26Z

frontend/model/state.js

@@ -455,7 +455,7 @@ const getters = {
  currentWelcomeInvite (state, getters) {
    const invites = getters.currentGroupState.invites
    const inviteId = Object.keys(invites).find(invite => invites[invite].creatorID === INVITE_INITIAL_CREATOR)
-    const expires = getters.currentGroupState._vm.authorizedKeys[inviteId].meta.expires
+    const expires = getters.currentGroupState._vm.invites[inviteId].expires


@corrideat Can you review this?

Let me know if this should be reverted.
But FYI, I made this change for two reasons

1) I noticed there is some other places where group's invites are used this way.

eg1.

group-income/frontend/model/contracts/shared/getters/group.js

Lines 133 to 136 in 9fd5e16

groupMembersPending (state, getters) {

const invites = getters.currentGroupState.invites

const vmInvites = getters.currentGroupState._vm.invites

const pendingMembers = Object.create(null)

eg2.

group-income/frontend/views/containers/group-settings/InvitationLinkModal.vue

Lines 48 to 54 in 9fd5e16

welcomeInviteSecret () {

const key = this.currentGroupState._vm.invites?.[this.currentWelcomeInvite.inviteId]?.inviteSecret

if (!key) {

console.error(`undefined key for welcomeInviteId: ${this.currentWelcomeInvite.inviteId}`)

return undefined

}

return key

eg3.

group-income/frontend/views/containers/proposals/ProposalItem.vue

Lines 308 to 313 in 9fd5e16

if (

this.currentGroupState._vm.invites?.[inviteKeyId]?.status === INVITE_STATUS.VALID &&

this.currentGroupState._vm.authorizedKeys?.[inviteKeyId] &&

this.currentGroupState._vm.authorizedKeys[inviteKeyId]._notAfterHeight == null &&

this.currentGroupState._vm.invites[inviteKeyId].inviteSecret

) {

2) It's hard to guess/know that getters.currentGroupState._vm.authorizedKeys has groups inivite, especially when it's the first time we look at this. getters.currentGroupState._vm.invites[inviteId] helps better in that it has invites in there and it feels less complex.

I think this change should be fine, provided that the key name starts with #inviteKey

To expand on this, some of the meta attributes are copied to the .invites field. I think it makes more sense to check under the dedicated .invites property, unless there's some reason not to. For example, if one wants access to the original attributes as they key was created, one would use _vm.authorizedKeys.

corrideat · 2024-09-27T12:21:34Z

frontend/controller/actions/group.js

@@ -173,7 +173,10 @@ export default (sbp('sbp/selectors/register', {
            permissions: [GIMessage.OP_KEY_REQUEST],
            meta: {
              quantity: 60,
-              expires: Date.now() + DAYS_MILLIS * INVITE_EXPIRES_IN_DAYS.ON_BOARDING,


See my comment for how not to set an expiry date (i.e., leave this blank). While you're at it, I'd also use the server time where possible (sbp('chelonia/time'))

SebinSong · 2024-09-27T20:58:11Z

@taoeffect
If we are going with what @corrideat suggested above,

In actions/group.js, if you skip the 'expires', the invite will not have an expiry time (although the modal will show 'This invite link expires on Invalid Date.' and the group settings will show it as active but 'expired').

We support this 'feature' already for the group creator to join the group (who joins using the CSK instead of an invite link). This way, the invite link doesn't show as 'used' which would be confusing.

I think it's better to discard this PR and write a new one, as it would require large amount of logic updates here.

Given the nature of how the logic related to group-invite has evolved, I think it's better for @corrideat to work on this issue. (I can focus on other tasks like image viewer)

what would you think?

taoeffect · 2024-09-27T21:15:48Z

You can put this PR on pause if you'd like, but Ricardo's plate is too full to take this issue on right now as he's focusing exclusively on push notifications and related work.

SebinSong · 2024-09-27T21:17:45Z

@taoeffect OK, let me close this PR for now and use it for a reference for a new PR I will write later then.

corrideat · 2024-09-28T13:43:14Z

@SebinSong I think it's better to discard this PR and write a new one, as it would require large amount of logic updates here.

I'm not sure using a 'never expires' should take so many logic changes (I honestly haven't checked), but if that's the case, mind that I only suggested it because 'never expires' is semantically closer to the intent. Setting it to some large value, like 20 years (or 100 years) is a valid (but less ideal) alternative.

In other words, my review wasn't meant to say exactly how it should be implemented.

SebinSong · 2024-09-28T16:46:34Z

@corrideat I think it does require redoing large portion of what I've done in this PR man..

Eg) NOTE 1] in #2351 (comment) has to be re-written and also we would need to search through the source code to find all places where something like below exists and makes sure the UI don't break.

We also wouldn't need to have 1. and 2. I've done in #2351 (comment) either, as there would be no case where app needs to update expires property of a group-invite, if anyone-can-join invite won't have this value in the first place.

So I think it's fair for me to decide to close it and it's better to be re-written from scratch (and no headache of figuring out which part to leave as is and which part needs to be updated again in this PR)

Greg said above you have a full table of TODOs now but go ahead and feel free to assign yourself to this issue and work on it if you want, while I'm focusing on other task at my hand too.

corrideat · 2024-09-28T17:06:34Z

Yeah, sure. I only wanted to point out that my suggestion was just that, in case it didn't come across like that. If you feel it'd be better to re-work it from scratch, then let's do that. It's unlikely I'll have time to work on this issue myself in the near future, but let's see.

SebinSong added 10 commits September 11, 2024 10:15

increase the life of anyone-invite-link

96e6592

update readableExpiryInfo method (incomplete)

abe1666

some refactoring in GroupMembers.vue and ProposalTemplate.vue

655bbfc

add generateInviteImmediately() method / make sure the invite works

c1f86a4

Merge remote-tracking branch 'origin/master' into sebin/task/#2341-an…

4c72344

…yone-can-join-invite-issue

Merge remote-tracking branch 'origin/master' into sebin/task/#2341-an…

f638fb7

…yone-can-join-invite-issue

fix the enter issue / update submit-btn text

2621b53

revert the invite life

f9ec6c5

update readable invite-life logic

95f1edc

update the period display expression again

43e557e

SebinSong self-assigned this Sep 16, 2024

SebinSong commented Sep 16, 2024

View reviewed changes

SebinSong added 2 commits September 18, 2024 13:37

Merge remote-tracking branch 'origin/master' into sebin/task/#2341-an…

8a437e4

…yone-can-join-invite-issue

update comment

c80ddeb

taoeffect requested changes Sep 19, 2024

View reviewed changes

corrideat reviewed Sep 22, 2024

View reviewed changes

Merge remote-tracking branch 'origin/master' into sebin/task/#2341-an…

709daac

…yone-can-join-invite-issue

implement gi.app/group/fixAnyoneCanJoinLink and use it

b31dd3e

SebinSong changed the title ~~#2341 Allow re-generation of anyone-can-join invite link~~ [WIP] #2341 Allow re-generation of anyone-can-join invite link Sep 24, 2024

taoeffect reviewed Sep 24, 2024

View reviewed changes

corrideat reviewed Sep 27, 2024

View reviewed changes

SebinSong closed this Sep 27, 2024

taoeffect mentioned this pull request Oct 4, 2024

Corrideat/task/#2341 anyone can join invite issue #2366

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] #2341 Allow re-generation of anyone-can-join invite link #2351

[WIP] #2341 Allow re-generation of anyone-can-join invite link #2351

SebinSong commented Sep 16, 2024

SebinSong Sep 16, 2024

SebinSong Sep 16, 2024

cypress bot commented Sep 16, 2024 •

edited

Loading

taoeffect left a comment •

edited

Loading

SebinSong commented Sep 20, 2024

corrideat Sep 22, 2024

taoeffect Sep 23, 2024

corrideat Sep 27, 2024

SebinSong commented Sep 23, 2024

SebinSong commented Sep 24, 2024 •

edited

Loading

taoeffect Sep 24, 2024

SebinSong Sep 24, 2024 •

edited

Loading

corrideat Sep 27, 2024

corrideat Sep 27, 2024

corrideat Sep 27, 2024

SebinSong commented Sep 27, 2024

taoeffect commented Sep 27, 2024

SebinSong commented Sep 27, 2024

corrideat commented Sep 28, 2024 •

edited

Loading

SebinSong commented Sep 28, 2024 •

edited

Loading

corrideat commented Sep 28, 2024

	groupMembersPending (state, getters) {
	const invites = getters.currentGroupState.invites
	const vmInvites = getters.currentGroupState._vm.invites
	const pendingMembers = Object.create(null)

	welcomeInviteSecret () {
	const key = this.currentGroupState._vm.invites?.[this.currentWelcomeInvite.inviteId]?.inviteSecret
	if (!key) {
	console.error(`undefined key for welcomeInviteId: ${this.currentWelcomeInvite.inviteId}`)
	return undefined
	}
	return key

	if (
	this.currentGroupState._vm.invites?.[inviteKeyId]?.status === INVITE_STATUS.VALID &&
	this.currentGroupState._vm.authorizedKeys?.[inviteKeyId] &&
	this.currentGroupState._vm.authorizedKeys[inviteKeyId]._notAfterHeight == null &&
	this.currentGroupState._vm.invites[inviteKeyId].inviteSecret
	) {

[WIP] #2341 Allow re-generation of anyone-can-join invite link #2351

[WIP] #2341 Allow re-generation of anyone-can-join invite link #2351

Conversation

SebinSong commented Sep 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cypress bot commented Sep 16, 2024 • edited Loading

taoeffect left a comment • edited Loading

Choose a reason for hiding this comment

Logic on how to upgrade expired anyone-can-join links

SebinSong commented Sep 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SebinSong commented Sep 23, 2024

SebinSong commented Sep 24, 2024 • edited Loading

Choose a reason for hiding this comment

SebinSong Sep 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SebinSong commented Sep 27, 2024

taoeffect commented Sep 27, 2024

SebinSong commented Sep 27, 2024

corrideat commented Sep 28, 2024 • edited Loading

SebinSong commented Sep 28, 2024 • edited Loading

corrideat commented Sep 28, 2024

cypress bot commented Sep 16, 2024 •

edited

Loading

taoeffect left a comment •

edited

Loading

SebinSong commented Sep 24, 2024 •

edited

Loading

SebinSong Sep 24, 2024 •

edited

Loading

corrideat commented Sep 28, 2024 •

edited

Loading

SebinSong commented Sep 28, 2024 •

edited

Loading