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: notifications fixes #230

Merged
merged 2 commits into from
Jul 17, 2024
Merged

fix: notifications fixes #230

merged 2 commits into from
Jul 17, 2024

Conversation

Tanya-atatakai
Copy link
Collaborator

@Tanya-atatakai Tanya-atatakai commented Jul 15, 2024

There are two issues with notifications:

  1. On the first run, a user should stake 20 OLAS. When the agent runs for the first time, we show a modal and a notification. The problem is that the notification shows "undefined" instead of "20" because we get the value from the existing service, which is actually not yet created when we call showNotification.

I changed the logic to get the minimumStakedAmountRequired from the template instead of from the service and removed it from the useReward since it's not used anywhere else. I also updated the NeedsFunds modal so it has the same source of truth.

The error:
image

After the fix:
image

2)The "first staking rewards earned" notification is shown for me multiple times in a row until the modal is closed. It happens due to the fact that useEffect has availableRewardsForEpochEth in its dependencies, which is slightly updated with every balance request that happens by interval. I used a ref here to get the first value, so the showNotification is only triggered once.

image

@Tanya-atatakai Tanya-atatakai force-pushed the tanya/notifications-fix branch from 4d96b58 to 9e197e6 Compare July 15, 2024 16:46
@Tanya-atatakai Tanya-atatakai changed the title fix: undefined in first time agent running notification fix: notifications fixes Jul 17, 2024
@Tanya-atatakai Tanya-atatakai requested review from truemiller, mohandast52 and oaksprout and removed request for truemiller July 17, 2024 12:05
@Tanya-atatakai Tanya-atatakai marked this pull request as ready for review July 17, 2024 12:06
@Tanya-atatakai Tanya-atatakai force-pushed the tanya/notifications-fix branch from 6d358a8 to c6c7082 Compare July 17, 2024 12:08
@Tanya-atatakai Tanya-atatakai merged commit 2b13bc2 into main Jul 17, 2024
4 checks passed
@Tanya-atatakai Tanya-atatakai deleted the tanya/notifications-fix branch July 17, 2024 12:40
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.

4 participants