gnosis / cowswap

šŸ® CowSwap: First Gnosis Protocol v2 UI
https://gnosis.io
GNU General Public License v3.0
114 stars 55 forks source link

[Affiliate] Wire up affiliate status check #944

Closed alfetopito closed 2 years ago

alfetopito commented 2 years ago

Summary

Using the component implemented on #943, show the appropriate message when user opens an affiliate link.

Once the affiliate link is loaded:

  1. Create metadata doc
  2. Create doc hash
  3. Post data to backend enpoint
  4. Based on response, display the appropriated message below
  5. Store on redux state the doc+hash sent if call was sucessful

Note: Messages are not final, feel free to propose better ones.

  1. Is it own link? (referral address matches connected wallet address) -> "Invalid code: cannot refer yourself"
  2. Has account already traded without referral code? (check will come from an endpoint still TBD) -> "Invalid code: referral only valid for new accounts"
  3. Has account already traded with a different referral code? (check will come from endpoint TBD) -> "Affiliate already active with a different referral code"
  4. In case network failure or any unknown reason -> Use default error message (check custom/operator/error/)
  5. Referall is valid and can be used -> "Affiliate program enabled"

Requirements

Out of scope

Additional context

alfetopito commented 2 years ago

I've created a flow chart with what was last discussed internally.

There won't be an endpoint for checking affiliate validity. Instead, a few checks will be done, defined in the diagram. The doc will also not need to be stored on local storage, only the referrer address and the appDataHash (appDocHash/appData, not sure on the name, feel free to suggest one if this doesn't make sense).

It might conflict with other tasks and what's written on this task (did not update it yet). Let use this as source of truth.

screenshot_2021-07-30_16-15-57.png

I'll update in case anything changes

ramirotw commented 2 years ago

@alfetopito so basically this new workflow put several tasks that were already done on hold:

We need to create a new ticket to handle the GET /v1/trades?owner=<address> endpoint. I'd say to update the getAppDataDoc method created in #960 for this and replace it by the trades endpoint.

elena-zh commented 2 years ago

Hi @alfetopito , I think that 1 condition is missing on the diagram: Has account already traded with a different referral code I believe, it might be covered by this condition, am I right? image

Also, I have one question: what should happen after a network failure or any unknown reason? Will a user be able to retry to use the affiliate link or so? At least, I think, we should add a 'Try again' button on the notification banner...

fairlighteth commented 2 years ago

@alfetopito Looking at your schema, I wonder what happens if the user decides to not connect a wallet (after following a referral url). Thinking about a scenario a users for reasons, abandons the screen.

Will it keep the affiliate ID in the local storage for next time the user comes back and connect their wallet?

alfetopito commented 2 years ago

@ramirotw

@alfetopito so basically this new workflow put several tasks that were already done on hold:

Unfortunately yes

We need to create a new ticket to handle the GET /v1/trades?owner=

endpoint. I'd say to update the getAppDataDoc method created in #960 for this and replace it by the trades endpoint.

Yes for a new issue, not sure it makes sense to re-use getAppDataDoc. I'd think of something like an utils function hasUserTraded(networkId, userAddress), encapsulating a getTrades call.


@elena-zh

Has account already traded with a different referral code. I believe, it might be covered by this condition, am I right?

Yes, if the user traded once, doesn't matter with which referral, we simplify and say a new code cannot be applied

Also, I have one question: what should happen after a network failure or any unknown reason? Will a user be able to retry to use the affiliate link or so? At least, I think, we should add a 'Try again' button on the notification banner...

Good point. In case of a failure anywhere in the flow (checking for trades OR uploading the metadata doc) we should notify the failure and do as you suggest.


@biocom

Will it keep the affiliate ID in the local storage for next time the user comes back and connect their wallet?

Initially I thought no. Bu maybe it makes sense to keep it until the wallet first connects. What do you think makes more sense?

ramirotw commented 2 years ago

Yes for a new issue, not sure it makes sense to re-useĀ getAppDataDoc. I'd think of something like an utils functionĀ hasUserTraded(networkId, userAddress), encapsulating aĀ getTradesĀ call.

Sorry I wasn't clear. I didn't mean reusing the method but having it replaced by a new method which for this use case works as a replacement. If not we'll need to remove it sooner than later.

alfetopito commented 2 years ago

Sorry I wasn't clear. I didn't mean reusing the method but having it replaced by a new method which for this use case works as a replacement. If not we'll need to remove it sooner than later.

Not sure I get it 100%, but I'll it up to your judgement.

fairlighteth commented 2 years ago

@alfetopito

Initially I thought no. Bu maybe it makes sense to keep it until the wallet first connects. What do you think makes more sense?

I think it makes sense to keep it in -> Somebody clicked on a referral url. They might not repeat that step (clicking on an url) -> but might continue just connecting their wallet at some point. It would be nice to still capture the referral for the affiliate ID (holder) in that scenario.

anxolin commented 2 years ago

Regarding the messages. Not sure when we use the term "referral" or "affiliate", but maybe we can try to be coherent.

Some proposals on the messages:

  1. Is it own link? (referral address matches connected wallet address) -> "Invalid code: cannot refer yourself"

I think for this message, we should be more positive, and think that the user followed his own link to make sure it works. One idea on the direction:

Your affiliate code works! Anyone new user following this link would credit you his trading volume

or Valid affiliate code: Anyone new user following this link would credit you his trading volume (if we want to make it more similar to the others)

Has account already traded without referral code? (check will come from an endpoint still TBD) -> "Invalid code: referral only valid for new accounts"

Like! Maybe Invalid affiliate code: Referral only ...

Has account already traded with a different referral code? (check will come from endpoint TBD) -> "Affiliate already active with a different referral code"

`Invalid affiliate code: The currently connected wallet is already part of the affiliate program"

Referall is valid and can be used -> "Affiliate program enabled"

Valid affiliate code: You can now do your first trade, to join the program

fairlighteth commented 2 years ago

There was a discussion somewhere on this (Slack), but if an existing user uses a new referral url, I believe we need another message/action:

Replace your existing referral code XXXX with XXXXX ?

We could then further elaborate that 'same benefits' or 'no new benefits' apply, but in case someone wants to switch referral.

anxolin commented 2 years ago

There was a discussion somewhere on this (Slack), but if an existing user uses a new referral url, I believe we need another message/action:

I think, in that case, the idea is just to show:

Valid affiliate code: You can now do your first trade, to join the program

And replace it. Do you think we need to be that explicit?

alfetopito commented 2 years ago

Giving the history of this task and the focus on the logic rather than the messages, I've created another task to define it https://github.com/gnosis/cowswap/issues/1343 @anxolin @biocom let's continue the discussion there.

Regarding the flow, this is the updated version I mentioned during the sync calls screenshot_2021-08-25_07-49-51

ramirotw commented 2 years ago

Closed by https://github.com/gnosis/cowswap/pull/1405