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

BUG: team plan pr comment shows pro pr format #1303

Closed
Tracked by #1302
codecovdesign opened this issue Feb 28, 2024 · 17 comments
Closed
Tracked by #1302

BUG: team plan pr comment shows pro pr format #1303

codecovdesign opened this issue Feb 28, 2024 · 17 comments
Assignees
Labels
bug Something isn't working in discovery The design, product, and specifications require refinement

Comments

@codecovdesign
Copy link
Contributor

codecovdesign commented Feb 28, 2024

Problem to solve

When testing / auditing the team plan, we identified the PR comment and it showed the full pro version:
Screenshot 2024-02-28 at 11 09 05 AM

additionally, the check appears:
Screenshot 2024-02-29 at 11 03 10 AM

example: https://github.com/drazisil-org/mco-server/pull/6
related org/repo: https://app.codecov.io/plan/gh/drazisil-org

Solution

Show intended PR comment: patch only w/ out details and no project check

@codecovdesign codecovdesign added the bug Something isn't working label Feb 28, 2024
@codecovdesign codecovdesign changed the title BUG: team plan pr comment shows pr BUG: team plan pr comment shows pro pr format Feb 28, 2024
@codecovdesign
Copy link
Contributor Author

@adrian-codecov just a note, @drazisil-codecov set this org to team plan in the DB when we were testing. Mentioning in case there is some other loose end in how we configed for testing for why it showed the pro plan version 🤔

@codecovdesign codecovdesign added the in discovery The design, product, and specifications require refinement label Feb 28, 2024
@adrian-codecov
Copy link

@codecovdesign, have you seen a customer in the wild that's properly team plan showcase this behavior? Just want to make sure it's not what you're saying, a manual DB check rather than a widespread problem, so just want to confirm before diving

@codecovdesign
Copy link
Contributor Author

codecovdesign commented Mar 6, 2024

@adrian-codecov I'll go double check the customer threads for screenshots; but in this case it was only the DB change. Is there a way we can recreate it from the start stripe purchase, then PR - this is recreate exact path?

EDIT: i found this one: codecov/feedback#54 (comment) but the issue is there we were experimenting at that time and it was unrelated to the team plan. Will keep thinking if there is another screenshot from somewhere 🤔

@codecovdesign
Copy link
Contributor Author

from bug sync @drazisil-codecov @adrian-codecov :

  • what happens when project is added to codecov.yaml + potentially changing default yaml to omit project
  • @drazisil-codecov will help troubleshoot

@drazisil-codecov
Copy link

@aj-codecov and @codecovdesign Can you link me to where the changes were implemented? I can't any plans checks in notify at all.

@adrian-codecov
Copy link

Gentle ping to see how I proceed w/ this ticket, or if it was a false positive while we were experimenting, @drazisil-codecov @codecovdesign (leaving AJ out since he's out)

@drazisil-codecov
Copy link

drazisil-codecov commented Mar 11, 2024 via email

@codecovdesign
Copy link
Contributor Author

@adrian-codecov we should proceed with investigation, it doesn't appear to be a false positive, but it would be ideal to confirm either way. per our bug sync , @drazisil-codecov mentioned she'd look for where we are changing the file and sounds like couldn't find.

@adrian-codecov
Copy link

I found this, https://github.com/codecov/worker/blob/355325bb19f15d7bcfcfc023c59a6cf0c9fe8730/services/notification/notifiers/mixins/message/__init__.py#L75-L76, which determines the type of comment we select. If layout_name == "newfiles" or layout_name == "condensed_files", then we return the comment that does patch_only metrics, https://github.com/codecov/worker/blob/355325bb19f15d7bcfcfc023c59a6cf0c9fe8730/services/notification/notifiers/mixins/message/sections.py#L446-L447. It didn't seem that this fn took plan type into account, do you know if we implemented this for the team plan back in the day? Trying to determine if this is a bug or if this is something that we never did

@codecovdesign
Copy link
Contributor Author

@adrian-codecov the intention was that it was to be implemented; I'm guessing it's a bug more than a missed issue, only since we did change it prior to team rollout, but that was unrelated to team and more experimental.

@adrian-codecov
Copy link

Gotchaa, ok that makes more sense, wanted to clarify the scope of the ticket, thanks!

@codecovdesign
Copy link
Contributor Author

sync with @adrian-codecov

@adrian-codecov
Copy link

@codecovdesign this change is in, lmk if you see this misbehaving!

@drazisil-codecov
Copy link

👋 A customer reported that 4931b3c9835633bc88510b64a5a82c3385eb6021 had this behavior on 29 April, @adrian-codecov

@adrian-codecov
Copy link

Sweet! Checked and this is a users-teamm customer so all tracks, thanks for confirming @drazisil-codecov

@drazisil-codecov
Copy link

Sweet! Checked and this is a users-teamm customer so all tracks, thanks for confirming @drazisil-codecov

Sorry, I meant they got a project status check. This is a case where it was explicitly set in the codeov.yml, but it's my understanding it should not on team plan, right?

@adrian-codecov
Copy link

Project status check != PR comment, see this ticket for fixing project status #1660, that should be addressed there. This issue was to fix what we displayed on the PR comment if they were/weren't team plan

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working in discovery The design, product, and specifications require refinement
Projects
None yet
Development

No branches or pull requests

3 participants