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

fix: Move the banner above summary, and fix the use of team hook #2859

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

RulaKhaled
Copy link
Contributor

@RulaKhaled RulaKhaled commented May 8, 2024

Description

The purpose of this PR is to left the onboarding banner up the summary. The banner should be visible for the user under the coverage tab

Notable Changes

  • Add isFirstPullRequest field within repository codecov-api#551
  • Added isFirstPR field under repo. We needed a quick way to know whether this repo has an unmerged first PR in Codecov
  • Call repo hook in coverage tab. No need to call team settings in coverage tab not sure why we ever did that
  • Create the onboarding banner under coverage tab

Screenshots

Screenshot 2024-05-07 at 1 54 28 PM

closes: codecov/engineering-team#1541

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

@RulaKhaled RulaKhaled marked this pull request as draft May 8, 2024 13:05
Copy link

codecov-public-qa bot commented May 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.48%. Comparing base (c1a8e81) to head (b41de14).
Report is 1 commits behind head on main.

✅ All tests successful. No failed tests found ☺️

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2859   +/-   ##
=======================================
  Coverage   98.47%   98.48%           
=======================================
  Files         878      879    +1     
  Lines       13026    13036   +10     
  Branches     3488     3433   -55     
=======================================
+ Hits        12828    12838   +10     
  Misses        194      194           
  Partials        4        4           
Files Coverage Δ
src/pages/RepoPage/CoverageTab/CoverageTab.jsx 100.00% <100.00%> (ø)
.../FirstPullRequestBanner/FirstPullRequestBanner.tsx 100.00% <100.00%> (ø)
...ubroute/FileExplorer/shared/RepoContentsResult.tsx 100.00% <100.00%> (ø)
src/services/repo/useRepo.tsx 94.44% <ø> (ø)

... and 1 file with indirect coverage changes

Components Coverage Δ
Assets 54.54% <ø> (ø)
Layouts 97.26% <ø> (ø)
Pages 99.28% <100.00%> (+<0.01%) ⬆️
Services 99.48% <ø> (ø)
Shared 99.68% <ø> (+<0.01%) ⬆️
UI 94.53% <ø> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c1a8e81...b41de14. Read the comment docs.

@codecov-notifications
Copy link

codecov-notifications bot commented May 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2859   +/-   ##
=======================================
  Coverage   98.47%   98.48%           
=======================================
  Files         878      879    +1     
  Lines       13026    13036   +10     
  Branches     3425     3501   +76     
=======================================
+ Hits        12828    12838   +10     
  Misses        194      194           
  Partials        4        4           
Files Coverage Δ
src/pages/RepoPage/CoverageTab/CoverageTab.jsx 100.00% <100.00%> (ø)
.../FirstPullRequestBanner/FirstPullRequestBanner.tsx 100.00% <100.00%> (ø)
...ubroute/FileExplorer/shared/RepoContentsResult.tsx 100.00% <100.00%> (ø)
src/services/repo/useRepo.tsx 94.44% <ø> (ø)

... and 1 file with indirect coverage changes

Components Coverage Δ
Assets 54.54% <ø> (ø)
Layouts 97.26% <ø> (ø)
Pages 99.28% <100.00%> (+<0.01%) ⬆️
Services 99.48% <ø> (ø)
Shared 99.68% <ø> (+<0.01%) ⬆️
UI 94.53% <ø> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c1a8e81...b41de14. Read the comment docs.

Copy link

codecov bot commented May 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.48%. Comparing base (c1a8e81) to head (b41de14).
Report is 1 commits behind head on main.

✅ All tests successful. No failed tests found.

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #2859   +/-   ##
=====================================
  Coverage   98.48   98.48           
=====================================
  Files        878     879    +1     
  Lines      13026   13036   +10     
  Branches    3470    3496   +26     
=====================================
+ Hits       12828   12838   +10     
  Misses       194     194           
  Partials       4       4           
Files Coverage Δ
src/pages/RepoPage/CoverageTab/CoverageTab.jsx 100.00% <100.00%> (ø)
.../FirstPullRequestBanner/FirstPullRequestBanner.tsx 100.00% <100.00%> (ø)
...ubroute/FileExplorer/shared/RepoContentsResult.tsx 100.00% <100.00%> (ø)
src/services/repo/useRepo.tsx 94.44% <ø> (ø)

... and 1 file with indirect coverage changes

Components Coverage Δ
Assets 54.54% <ø> (ø)
Layouts 97.26% <ø> (ø)
Pages 99.28% <100.00%> (+<0.01%) ⬆️
Services 99.48% <ø> (ø)
Shared 99.68% <ø> (+<0.01%) ⬆️
UI 94.53% <ø> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c1a8e81...b41de14. Read the comment docs.

@codecov-qa
Copy link

codecov-qa bot commented May 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.48%. Comparing base (c1a8e81) to head (b41de14).
Report is 1 commits behind head on main.

✅ All tests successful. No failed tests found.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2859   +/-   ##
=======================================
  Coverage   98.47%   98.48%           
=======================================
  Files         878      879    +1     
  Lines       13026    13036   +10     
  Branches     3488     3496    +8     
=======================================
+ Hits        12828    12838   +10     
  Misses        194      194           
  Partials        4        4           
Files Coverage Δ
src/pages/RepoPage/CoverageTab/CoverageTab.jsx 100.00% <100.00%> (ø)
.../FirstPullRequestBanner/FirstPullRequestBanner.tsx 100.00% <100.00%> (ø)
...ubroute/FileExplorer/shared/RepoContentsResult.tsx 100.00% <100.00%> (ø)
src/services/repo/useRepo.tsx 94.44% <ø> (ø)

... and 1 file with indirect coverage changes

Components Coverage Δ
Assets 54.54% <ø> (ø)
Layouts 97.26% <ø> (ø)
Pages 99.28% <100.00%> (+<0.01%) ⬆️
Services 99.48% <ø> (ø)
Shared 99.68% <ø> (+<0.01%) ⬆️
UI 94.53% <ø> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c1a8e81...b41de14. Read the comment docs.

Copy link

codecov bot commented May 8, 2024

Bundle Report

Changes will increase total bundle size by 378 bytes ⬆️

Bundle name Size Change
gazebo-production-array-push 6.61MB 378 bytes ⬆️

@codecov-staging
Copy link

codecov-staging bot commented May 8, 2024

Bundle Report

Changes will increase total bundle size by 378 bytes ⬆️

Bundle name Size Change
gazebo-staging-array-push 6.61MB 378 bytes ⬆️

@codecov-releaser
Copy link
Contributor

codecov-releaser commented May 8, 2024

✅ Deploy preview for gazebo ready!

Previews expire after 1 month automatically.

Commit Created Cloud Enterprise
8cefc74 Wed, 08 May 2024 13:16:06 GMT Expired Expired
e94c7d4 Tue, 14 May 2024 08:47:30 GMT Expired Expired
d916112 Tue, 14 May 2024 09:58:49 GMT Expired Expired
20f6c1e Tue, 14 May 2024 11:21:02 GMT Expired Expired
6efa912 Thu, 16 May 2024 16:44:46 GMT Expired Expired
defe05a Fri, 17 May 2024 11:05:22 GMT Expired Expired
b41de14 Fri, 17 May 2024 11:26:41 GMT Cloud Enterprise

@RulaKhaled RulaKhaled marked this pull request as ready for review May 14, 2024 11:22
@nicholas-codecov nicholas-codecov self-requested a review May 17, 2024 10:49
Copy link
Contributor

@nicholas-codecov nicholas-codecov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small request, that I think should help the users out

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

m: I know you're moving the banner to the top, however I think it's still worth showing some form of an error message down here, instead of just an empty table, users "shouldn't" miss the banner at the top, but they might, so I think we should still leave some error message down in the table so they're at least aware of some issue occurring.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you suggest we leave the banner there as well, or loop in Kyle for a new copy?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uhh i think new copy would probably be best imo, i would show something similar to how we currently display the other error messages ... the banner always felt out of place

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or if you feel comfortable coming up with something, i think that'll be alright as well, it could be the same copy that's in the banner ... just not in the banner

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

Successfully merging this pull request may close these issues.

BUG: banner not showing in right location
3 participants