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

Fix: Make our app automatically aware of new Colony Contributors #3801

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rumzledz
Copy link
Contributor

@rumzledz rumzledz commented Nov 28, 2024

Description

Issue #3622 is a bit misleading. It's not that the app doesn't ever show the new contributor. It's just that on master, you'd have to reload the app to make our app refetch our contributors list.

So I added a subscription logic to make the app automatically aware that a new contributor has been added to a Colony.

And the result is nothing short of spectacular ✨

app-aware-new-contributors

Testing

  1. Connect Leela's wallet
  2. Create a new Colony using this script:
node scripts/create-colony-url.js
  1. Visit your new Colony
  2. Stay on the Dashboard
  3. Open an incognito tab
  4. Connect Amy's wallet (Wallet 2)
  5. Generate an invite code for the new Colony:
query MyQuery {
  getColonyByName(name: "your-new-colony's-name") { <<-- NOTE: Update this with your new Colony's name
    items {
      colonyMemberInviteCode
    }
  }
}
  1. The result should be like this:
{
  "data": {
    "getColonyByName": {
      "items": [
        {
          "colonyMemberInviteCode": "cb488112-49af-4fd8-a666-5a3c71d35f1e"
        }
      ]
    }
  }
}
  1. Copy the colonyMemberInviteCode field value i.e. "cb488112-49af-4fd8-a666-5a3c71d35f1e"
  2. On the same Incognito tab, make sure that Amy's wallet is still connected
  3. Visit http://localhost:9091/invite/{new-colony-name}/{colonyMemberInviteCode}
  4. Proceed to join the Colony
  5. Once you've successfully joined the Colony, switch back to the non-incognito window
  6. VERY IMPORTANT: Please do not refresh the page
  7. Open the Simple Payment form
  8. Click the recipient field
  9. Verify that your new user is visible on the list of Recipients
  10. Visit the Members page
  11. Verify that your new user is visible under the Followers tab

Resolves #3632

@rumzledz rumzledz self-assigned this Nov 28, 2024
@rumzledz rumzledz force-pushed the fix/3632-make-the-app-aware-of-new-contributors branch from b8b9482 to f83d288 Compare November 28, 2024 08:18
@rumzledz rumzledz marked this pull request as ready for review November 28, 2024 08:19
@rumzledz rumzledz requested a review from a team as a code owner November 28, 2024 08:19
@@ -119,7 +120,7 @@ const MemberContextProvider: FC<PropsWithChildren> = ({ children }) => {
data: memberSearchData,
loading,
fetchMore,
refetch,
refetch: refetcColonyContributors,
Copy link
Contributor

Choose a reason for hiding this comment

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

refetcH

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AH!

@rumzledz rumzledz requested review from iamsamgibbs and a team November 28, 2024 09:22
@rumzledz rumzledz force-pushed the fix/3632-make-the-app-aware-of-new-contributors branch from f83d288 to a949d7a Compare November 28, 2024 09:23
@rumzledz rumzledz force-pushed the fix/3632-make-the-app-aware-of-new-contributors branch from a949d7a to fac70af Compare November 28, 2024 09:40
Copy link
Contributor

@Nortsova Nortsova left a comment

Choose a reason for hiding this comment

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

Hey @rumzledz , thank you for this nice fix ✨

Simple payments works as expected 🙌
image

However on Members page the Followers count was not updated:
image

And dashboard members count was not updated as well:
image

Thank you!

@rumzledz
Copy link
Contributor Author

Those are irrelevant to the raised issue but sure, I'll fix it too 😂

@rumzledz rumzledz requested review from Nortsova and a team November 28, 2024 14:07
@rumzledz
Copy link
Contributor Author

@Nortsova There! Irrelevant fix pushed 😂 🚀

let timeout: NodeJS.Timeout;

if (newColonyContributor) {
// It looks hacky, but we need the timeout to ensure that opensearch has been updated before we refetch.
Copy link
Contributor

@iamsamgibbs iamsamgibbs Nov 28, 2024

Choose a reason for hiding this comment

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

I know this fixes such a small edge case that it doesn't really matter too much, but is a 2000 timeout reliable for opensearch having updated? Does it reliably take less than 2 seconds?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @iamsamgibbs ! I tried to do 1 second and was able to notice an issue in like 1 out of 10 times. But I never had an issue with 2, so I stuck with 2, which also aligns with the colony contributor refetch done on the MemberContextProvider. There’s also a little loading window before the user gets to the colony after joining so it shouldn’t be too much of an issue 😄

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.

[Payment -QA] The new follower isn't displayed in Simple Payment Recipient list
3 participants