Skip to content
This repository has been archived by the owner on Jun 24, 2022. It is now read-only.

[Affiliate] Close all affiliate banners after a successful trade #1748

Merged
merged 13 commits into from
Nov 5, 2021

Conversation

matextrem
Copy link
Contributor

Summary

Fixes #1707

Remove info banner when order is recently fulfilled.

To Test

  1. Navigate through a referral link with a new account.
  • You will see Valid Affiliate code: You can now do your first trade to join the program banner message
  1. Under the new account do the 1st trade in the mainnet.
  • Information banner will disappear

@matextrem matextrem added Bug Something isn't working Protofire Handled by Protofire development team labels Nov 2, 2021
@matextrem matextrem self-assigned this Nov 2, 2021
@matextrem matextrem changed the title Check for recent orders and update banner message [Affiliate] Close all affiliate banners after a successful trade Nov 2, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Nov 2, 2021

  • 🔭 GP Swap: Gnosis Protocol v2 Swap UI

Copy link

@elena-zh elena-zh left a comment

Choose a reason for hiding this comment

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

Hi @matextrem ,
Now the banner disappears when sign an order.
However, along with the banner, an order info is not displayed in the activity side bar.
no history
no history 2

Moreover, the banner disappears on the 'Sign' order event, and it is not correct, as the order can be 'Expired' or 'Cancelled', and will not be included into trades list.

Also, info banners do not appear for a while for the account even if I follow another referral link.

So, my expected result for the issue is:

  1. Close info banner as soon as a user places a successful trade.
  2. Do not clear out recent activities from the local storage

Copy link
Contributor

@alfetopito alfetopito left a comment

Choose a reason for hiding this comment

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

A few comments regarding the code, did not get to test the behaviour

src/custom/components/AffiliateStatusCheck/index.tsx Outdated Show resolved Hide resolved
src/custom/components/AffiliateStatusCheck/index.tsx Outdated Show resolved Hide resolved
@anxolin
Copy link
Contributor

anxolin commented Nov 3, 2021

I won't include this PR in release/1.4 since it has unaddressed issues. One the release is closed, pls repoint this PR to develop branch

@matextrem matextrem changed the base branch from release/1.4.0 to develop November 4, 2021 03:53
Copy link

@elena-zh elena-zh left a comment

Choose a reason for hiding this comment

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

In this PR I see a lot of requests sent to the BE.
As the result, BE starts blocking requests and fails to load profile data
https://watch.screencastify.com/v/vdi1v61D3xta8nkV7U8M

Copy link
Contributor

@alfetopito alfetopito left a comment

Choose a reason for hiding this comment

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

Looks good, one minor comment regarding the import path

import { ButtonSecondary } from 'components/Button'
import { OrderStatus } from '@src/custom/state/orders/actions'
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it work with the previous import path?

@elena-zh
Copy link

elena-zh commented Nov 5, 2021

Hi @matextrem , now I still see a previous banner after a successful trade. It remains to be displayed until I reload the page.
As soon as I reload the page, I see 'You have already traded' banner.
Can we just close all banners after a successful trade and safe its state for an account until a user enters a link with another referral code?
still displayed

The video is recorded for another issue I'm going to report, but here you can see the described behavior above
https://watch.screencastify.com/v/4eEJEQNGCoT0ddvJtlNA

@matextrem
Copy link
Contributor Author

@elena-zh I updated the CR in order to dismiss the banner after a trade. The second point you mention I think it should not be part of this CR since it is more related to the one made by Maria #1756

Copy link

@elena-zh elena-zh left a comment

Choose a reason for hiding this comment

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

Changes LGTM now.

@alfetopito alfetopito added the Auto-merge PRs with this tag will be automatically merged when approved and CI succeeds label Nov 5, 2021
@mergify mergify bot merged commit 2c121be into develop Nov 5, 2021
@alfetopito alfetopito deleted the 1707/invalid-code-message branch November 5, 2021 19:48
ramirotw pushed a commit that referenced this pull request Nov 10, 2021
Fixes #1707

Remove info banner when order is recently fulfilled.

1. Navigate through a referral link with a new account.
- You will see `Valid Affiliate code: You can now do your first trade to join the program` banner message
2. Under the new account do the 1st trade in the mainnet.
- Information banner will disappear
ramirotw pushed a commit that referenced this pull request Nov 10, 2021
Fixes #1707

Remove info banner when order is recently fulfilled.

1. Navigate through a referral link with a new account.
- You will see `Valid Affiliate code: You can now do your first trade to join the program` banner message
2. Under the new account do the 1st trade in the mainnet.
- Information banner will disappear
@ramirotw ramirotw mentioned this pull request Nov 10, 2021
nenadV91 pushed a commit that referenced this pull request Nov 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Auto-merge PRs with this tag will be automatically merged when approved and CI succeeds Bug Something isn't working Protofire Handled by Protofire development team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Affiliate] Message about invalid code is displayed after a successful 1st trade
5 participants