Giveth / giveth-planning

GNU General Public License v3.0
28 stars 5 forks source link

Figure out why Donor wallets are wrong in Autopilot #976

Closed ahmadabugosh closed 7 months ago

ahmadabugosh commented 1 year ago

@WhyldWanderer Brought to my attention that a lot of the Donor wallet addresses in Autopilot are incorrect. I believe this has happened in the last three weeks (since it was not the case when we sent the last GIVbacks email). It's also not the case with all records (for example mine and Ashley's are still correct) but many are wrong (like Griff's and Lauren's)

I recommend we

1) Look through Autopilot and see what automation or human error could have caused this issue (I'm doing this, but any help would be appreciated).

2) We can roll back the changes to two weeks ago. I have a list I downloaded of Donor wallets from the last few GIVbacks rounds, so we can always go and fix them.

However, we do need to look into why this happened so it doesn't keep happening.

@laurenluz @GriffGreen @divine-comedian

divine-comedian commented 1 year ago

@CarlosQ96 made some changes recently to the way we send events from our dapp into segment - this information is then forwarded to autopilot - could it be that some information is being sent or captured incorrectly.

CarlosQ96 commented 1 year ago

The new segment changes are currently only active on the new features like MandatoryUpdates, so that isn't the issue.

But I have two leads:

  1. Segment Identify function is used to tie the email to a user. User changed it's email -> backend attempted to send update email with segment thirdparty code -> fails -> old email remained in segment.
  2. Identify User doesn't send the wallet, we have to add it to impact graph.

Question: How does segment tie the address to the email? I'm looking in the backend but can't find it, like I said Segment.identify is not sending the address.

@divine-comedian

ahmadabugosh commented 1 year ago

Hey @CarlosQ96 @divine-comedian ,

After doing some digging, I'm pretty confident that the Autopilot Donor wallet addresses are being changed for Project owners into addresses of their Donors from Segment.

For example, @laurenluz received a donation from this address for a donation to the Giveth matching pool, triggering her address to be changed to that Donor address in Autopilot.

I think the culprit could be the Segment implementation of "Donation Received":

Screen Shot 2022-11-03 at 2 44 26 PM

So I think the problem is that when a new donation is received it overrides the Donor Wallet Address of the Project owner.

Any idea of how to fix this?

CarlosQ96 commented 1 year ago

Thanks! A lot @ahmadabugosh I will look into it. This is likely the reason. One fix is to the change the mapping for another variables instead of "DonorWalletAddress" for this

divine-comedian commented 1 year ago

Thanks! A lot @ahmadabugosh I will look into it. This is likely the reason. One fix is to the change the mapping for another variables instead of "DonorWalletAddress" for this

I think if when we send information to segment from the impact-graph we could rename or remove the property fromWalletAddress in "Donation Received' that would resolve the issue.

I don't think in the donation received email we even need to keep track of the donor address.

we're already capturing the donor address in donation made

laurenluz commented 1 year ago

Hey! this is right for me for this round, FYI:

image

CarlosQ96 commented 1 year ago

@ahmadabugosh @divine-comedian merged to production the hotfix removing the fromWalletAddress from event "Donation Received".

ahmadabugosh commented 1 year ago

hey @divine-comedian @CarlosQ96 @WhyldWanderer ,

I think we still need to solve this :(

When I export a csv to do a vlookup from Autopilot to compare the Donor Wallet Addresses to get the Donor Email, I still get wrong values, and it's making sending the GIVbacks email into a much longer task than it needs to be.

For example, because @GriffGreen recently donated to the Giveth house, @franculio 's Donor Address value in Autopilot got changed again to Griff's.

As a temporary solution, I've been checking the addresses one by one and confirming them (and deleting ones I know are wrong), but this still has some problems.

1) I think the "Donation Received" journey is still changing the Donor Addresses when it shouldn't. It's hard to say but since I sent the last GIVbacks email, it appears some of them got changed back.
2) Even if we solve that problem, now most of the Donor addresses appear to be wrong (if a donor donated to multiple projects, now those projects all have the same donor address so it's hard to know which one is the real one).

How do you think we can solve this?

Is there any other place where we're storing a pair of email addresses along with the Donor wallet address (maybe in the backend somewhere)?

If not, then probably we should make sure that "Donation Received" is 100% not changing the Donor wallet address anymore, and if that's the case, then we should probably clear out most wallet addresses (for any projects) and start fresh.

MoeNick commented 1 year ago

hmmm @CarlosQ96 do you have any idea how we can make it correct? @ahmadabugosh I bring this issueback to the product backlog and put a high priority for it.

ahmadabugosh commented 1 year ago

@CarlosQ96 gave me a list of the addresses linked to the email addresses for donors from the backend and I've been manually updating them in Autopilot when I send the GIVbacks email.

I'll keep monitoring it, and let you know if I notice if the issue still persists.

oyealmond commented 7 months ago

Hi @divine-comedian tagging you here for you to not forget this issue. Let me know if we can support.

ahmadabugosh commented 7 months ago

@oyealmond This should be solved with the move to Ortto.

Just one thing @divine-comedian, can we get a clean list of emails associated with donor addresses from the database (maybe from @RamRamez) and then override the ones currently in Ortto to make sure they're all good going forward?

After we do that, we can close this issue.

divine-comedian commented 7 months ago

Sure @RamRamez - can you pull a csv from the DB that includes all userIds that have made at least one donation,

Fields we should include in the response:

RamRamez commented 7 months ago

@divine-comedian I sent you the list in DM

divine-comedian commented 7 months ago

@krmet @ahmadabugosh - I imported the CSV that ramin shared into the CRM - you can find them by the Donor tag image