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

Spike: Understand why onboarded student still considered pending on team [8,13,0.62] #2372

Closed
stenington opened this issue Feb 6, 2020 · 13 comments

Comments

@stenington
Copy link
Contributor

stenington commented Feb 6, 2020

Describe the bug
We've seen a situation where a student is considered pending on a team until they complete onboarding, but when you look at their account, onboarding is complete. Somehow the onboarded flag that captures that status failed to update correctly.

If you look up a team with such a member in the admin, it will show them pending with "X still needs to:" and then no steps listed below.

Screen Shot 2020-02-05 at 1 45 15 PM

To Reproduce
Steps to reproduce the behavior:

This one is tricky to reproduce because we don't know exactly how it happens. The onboarded flag should get rechecked any time the profile is updated, so to force this situation on staging, I think you'd have to directly change the onboarded column in the database (rather than the console).

Expected behavior
We expect that if a team member is still pending, they haven't completed some piece of onboarding.

Notes on resolving
It seems to be sufficient to do something like student_profile.touch to re-trigger the onboarding checks. It's also possible that if a user changed some piece of information on their profiles themselves that that might trigger the recalc, though I'm not 100% positive.

@stenington
Copy link
Contributor Author

We saw this once in #2369 (and I remember it happening last season, though I'm not sure how often).

@rsgonzal
Copy link
Collaborator

We have another student experiencing the same (I think) issue
Screen Shot 2020-02-12 at 3 01 10 PM

@rsgonzal rsgonzal changed the title Onboarded student still considered pending on team Onboarded student still considered pending on team [13,x,x] Feb 13, 2020
@rsgonzal rsgonzal changed the title Onboarded student still considered pending on team [13,x,x] Onboarded student still considered pending on team [20?,x,x] Feb 13, 2020
@hellafitz hellafitz changed the title Onboarded student still considered pending on team [20?,x,x] Spike: Onboarded student still considered pending on team [20,13,1.54] Feb 13, 2020
@stenington stenington changed the title Spike: Onboarded student still considered pending on team [20,13,1.54] Spike: Understand why onboarded student still considered pending on team [20,13,1.54] Feb 13, 2020
@stenington
Copy link
Contributor Author

stenington commented Feb 13, 2020

If understanding why it's happening becomes a black hole, refocus spike on proposing a way to detect and recover from this situation without a human in that loop. Goal is to get this to stop happening within a couple iterations.

@rsgonzal
Copy link
Collaborator

rsgonzal commented Feb 21, 2020

Noting we worked on #2388 during it-16, for an automated daily check of onboarded status (but probably wouldn't scale very effectively, relies on a small number of users, also this is a band-aid not a root cause solution to why this is happening - which we don't know)

@hellafitz hellafitz changed the title Spike: Understand why onboarded student still considered pending on team [20,13,1.54] Spike: Understand why onboarded student still considered pending on team [8,13,0.62] Feb 26, 2020
@rsgonzal rsgonzal added it-18 and removed it-17 labels Mar 11, 2020
@rsgonzal rsgonzal added it-19 and removed it-18 labels Mar 25, 2020
@rsgonzal rsgonzal added it-22 and removed it-19 labels May 6, 2020
@stenington
Copy link
Contributor Author

At this point we're looking for either

  1. steps to reproduce this bug,
  2. an explanation of how it happens, in the case that it can reliably be reproduced, or
  3. admission of defeat (i.e. we haven't been able to explain it in the allotted time).

In the first two cases we can talk about a fix, in the last case we might want to talk about setting up some monitoring of the problem so we can keep it in check.

@shaun-technovation shaun-technovation self-assigned this Sep 10, 2020
@shaun-technovation
Copy link
Contributor

I was finally able to reproduce this one locally - where the onboarded flag is false. but can_be_onboarded? is true. 🤯 🤕 😁

One way this can happen is when we run reset_for_season, this will reset the student's onboarded flag (set it to false).

So, for returning students (who were previously fully onboarded), the onboarded flag will be set to false, but can_be_on_boarded? will be true (b/c they were already onboarded).

Roxana was able to help me confirm this on QA, the account/student student@student.com was experiencing this issue, and they are a returning student.

@shaun-technovation
Copy link
Contributor

There isn't a PR for this, but I moved it to the "Code Review" column, maybe you want to read/confirm, @stenington ?

@stenington
Copy link
Contributor Author

I see how reset_for_season could be a problem if you're messing with the season rollover date after the season has already started, but in the general case parental consents should automatically expire on the rollover date, wouldn't they? Which means can_be_marked_onboarded? will be false, matching the onboarded flag set to false.

But anyway, I think my takeaway is that the problem is subtle and for now, the onboarded flag isn't 100% trustworthy. I'm not sure we need to take this ticket any farther.

@shaun-technovation
Copy link
Contributor

in the general case parental consents should automatically expire on the rollover date, wouldn't they?

Hmm, I'm not seeing where parental consent forms automatically expire (I found it for consent forms), when you get a moment, can you point me to where this happens? Think I'm missing part of the puzzle.

@stenington
Copy link
Contributor Author

Sure!

  • can_be_marked_onboarded checks signed_parental_consent.present? here
  • signed_parental_consent is a has_one on current.signed here
  • current is a scope defined in seasoned.rb that filters down to items marked for the current season here
  • and the current season changes on the same day reset_for_seasons runs here and here

@stenington
Copy link
Contributor Author

My wording was poor, the consent doesn't expire so much as the student profile will start looking for one in the new season, despite the past season consent still being there.

@shaun-technovation
Copy link
Contributor

My wording was poor, the consent doesn't expire so much as the student profile will start looking for one in the new season, despite the past season consent still being there.

Right, gotcha, not so much expiring, but I see what you mean.

But anyway, I think my takeaway is that the problem is subtle and for now, the onboarded flag isn't 100% trustworthy. I'm not sure we need to take this ticket any farther.

I agree, the only way I found (after days of trying) to reproduce this bug seems very unlikely in the real world. So yeah, I think I've spent all the time I want to on it for now.

Guess that means opting for option 3:

admission of defeat (i.e. we haven't been able to explain it in the allotted time).

😫 😩

I think for the next steps I'll close this one, and open another one for some sort of monitoring/logging or something to hopefully give us some insights into the issue.

@shaun-technovation
Copy link
Contributor

I added #2671 to help further this one along. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants