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

[HOLD for payment 2023-12-07] [HOLD for payment 2023-10-13] [HOLD for payment 2023-07-14] Onyx migration for personalDetails changes #21454

Closed
8 of 9 tasks
puneetlath opened this issue Jun 23, 2023 · 52 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Weekly KSv2

Comments

@puneetlath
Copy link
Contributor

puneetlath commented Jun 23, 2023

Part of "Secure Logins in NewDot" Project

As part of this project, we started sending new data to Onyx that was accountID-based (e.g. personalDetailsList, participantAccountIDs, etc). We did not stop sending the old data, but we will be soon.

As a cleanup item, we should add an Onyx migration for two reasons:

  1. To remove stale data from clients (e.g. to remove the personalDetails object)
  2. To fix stale local data that will cause the app to break (e.g. old reportActions that don't have participantAccountIDs)

We should have this ready to go-live after https://github.com/Expensify/Expensify/issues/294647 is done.

This migration will be handled in stages:

Before updating the back-end:

  • Migrate reportActions that don't have accountID-based data to include it (without removing email-based data)

Once back-end has been updated to stop sending email-based data:

@puneetlath puneetlath added the Daily KSv2 label Jun 23, 2023
@puneetlath puneetlath self-assigned this Jun 23, 2023
@puneetlath puneetlath changed the title Migration for personalDetails changes Onyx migration for personalDetails changes Jun 23, 2023
@puneetlath

This comment was marked as duplicate.

@melvin-bot melvin-bot bot added the Reviewing Has a PR in review label Jun 26, 2023
@puneetlath
Copy link
Contributor Author

@s77rt if you want, you can C+ all of them. There will probably be at least 3 or 4 PRs.

@s77rt
Copy link
Contributor

s77rt commented Jun 26, 2023

@puneetlath That would be great

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Daily KSv2 labels Jul 3, 2023
@melvin-bot melvin-bot bot changed the title Onyx migration for personalDetails changes [HOLD for payment 2023-07-10] Onyx migration for personalDetails changes Jul 3, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 3, 2023

Reviewing label has been removed, please complete the "BugZero Checklist".

@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Jul 3, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 3, 2023

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.35-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2023-07-10. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

  • External issue reporter
  • Contributor that fixed the issue
  • Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

  • Merged PR within 3 business days of assignment - 50% bonus
  • Merged PR more than 9 business days after assignment - 50% penalty

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Jul 7, 2023
@melvin-bot melvin-bot bot changed the title [HOLD for payment 2023-07-10] Onyx migration for personalDetails changes [HOLD for payment 2023-07-14] [HOLD for payment 2023-07-10] Onyx migration for personalDetails changes Jul 7, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 7, 2023

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.37-7 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2023-07-14. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

  • External issue reporter
  • Contributor that fixed the issue
  • Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

  • Merged PR within 3 business days of assignment - 50% bonus
  • Merged PR more than 9 business days after assignment - 50% penalty

@Beamanator
Copy link
Contributor

Planning to help with Update migration to remove email-based data from reports once we have all of the emails out of reports 👍

@s77rt
Copy link
Contributor

s77rt commented Jul 12, 2023

@Beamanator We have PR for that already #22432

@puneetlath
Copy link
Contributor Author

@Beamanator is working on the back-end that should unblock the last couple pieces. Hopefully we're very close to the finish line here 🤞🏾

@melvin-bot melvin-bot bot removed the Overdue label Oct 18, 2023
@melvin-bot melvin-bot bot added the Overdue label Oct 26, 2023
@puneetlath
Copy link
Contributor Author

Getting closer!

@melvin-bot melvin-bot bot removed the Overdue label Nov 1, 2023
@melvin-bot melvin-bot bot added the Overdue label Nov 9, 2023
@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Weekly KSv2 Overdue labels Nov 21, 2023
@puneetlath
Copy link
Contributor Author

Next (last?) PR is here: #31664

@puneetlath
Copy link
Contributor Author

I believe we're officially done with this now that the last PR was merged 🎉

@Beamanator @s77rt do you agree that we're done? Or are we still missing something?

@Beamanator
Copy link
Contributor

Beamanator commented Nov 29, 2023

I thinkkkk yes we can finally close this one out! (if payment is complete)

@s77rt
Copy link
Contributor

s77rt commented Nov 29, 2023

  • Update migration not to run when it isn't needed

What about this one?

@puneetlath
Copy link
Contributor Author

Hmm, I can't remember what I meant by that. Is there a way we can know that the migration doesn't need to run and skip it that y'all can think of?

@Beamanator
Copy link
Contributor

haha fair point - i don't think we'd have any way of knowing that it already ran unless we set some kind of NVP, but we'd have to have it run for every device where you had logged in to your account - so I don't think we need to do that now, we may need to if we start running too many migrations that end up taking a long time or something?

@s77rt
Copy link
Contributor

s77rt commented Nov 30, 2023

Agree ^ I think we are good to close for now

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Nov 30, 2023
@melvin-bot melvin-bot bot changed the title [HOLD for payment 2023-10-13] [HOLD for payment 2023-07-14] Onyx migration for personalDetails changes [HOLD for payment 2023-12-07] [HOLD for payment 2023-10-13] [HOLD for payment 2023-07-14] Onyx migration for personalDetails changes Nov 30, 2023
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Nov 30, 2023
Copy link

melvin-bot bot commented Nov 30, 2023

Reviewing label has been removed, please complete the "BugZero Checklist".

Copy link

melvin-bot bot commented Nov 30, 2023

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.5-7 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2023-12-07. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

  • External issue reporter
  • Contributor that fixed the issue
  • Contributor+ that helped on the issue and/or PR

For reference, here are some details about the assignees on this issue:

@puneetlath
Copy link
Contributor Author

Ok, I count a total of 7 PRs which would be $3500. Does that sound right?

@puneetlath
Copy link
Contributor Author

Ok that sounds good. Thank you for being so honest and straightforward. Offer here: https://www.upwork.com/nx/wm/offer/27918877

@s77rt
Copy link
Contributor

s77rt commented Dec 1, 2023

Accepted! Thanks!

@puneetlath
Copy link
Contributor Author

Alright paid. Closing this out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Weekly KSv2
Projects
None yet
Development

No branches or pull requests

3 participants