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

[Feature] Add process number to tables #10763

Merged
merged 9 commits into from
Jun 28, 2024

Conversation

yonikid15
Copy link
Contributor

@yonikid15 yonikid15 commented Jun 21, 2024

🤖 Resolves #10695

👋 Introduction

  • Adds process number after pool name on the view candidate page - Other processes section.
  • Adds process number after pool name on Candidate search page.
  • The process number should be filterable on Candidate search table.

🧪 Testing

Assist reviewers with steps they can take to test that the PR does what it says it does.

  1. Login as admin@test.com
  2. Go to Candidate search page and ensure process number is there and is filterable.
  3. View a candidate in a pool and open Other processes accordion to reveal table. Ensure the table has the process number after name of pool.

📸 Screenshot

image

image

@codecov-commenter
Copy link

codecov-commenter commented Jun 21, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 21.42857% with 11 lines in your changes missing coverage. Please review.

Project coverage is 38.27%. Comparing base (5792eb4) to head (dd8fe66).
Report is 2 commits behind head on main.

Files Patch % Lines
api/app/Models/PoolCandidate.php 0.00% 7 Missing ⚠️
...web/src/components/PoolCandidatesTable/helpers.tsx 25.00% 3 Missing ⚠️
...src/components/PoolStatusTable/PoolStatusTable.tsx 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #10763      +/-   ##
============================================
- Coverage     38.28%   38.27%   -0.02%     
- Complexity     1652     1654       +2     
============================================
  Files          1021     1021              
  Lines         31191    31221      +30     
  Branches       6509     6523      +14     
============================================
+ Hits          11943    11951       +8     
- Misses        19081    19236     +155     
+ Partials        167       34     -133     
Flag Coverage Δ
integrationtests 67.81% <0.00%> (-0.08%) ⬇️
unittests 31.16% <42.85%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@vd1992
Copy link
Contributor

vd1992 commented Jun 24, 2024

Little confused here, which is it supposed to be?

The issue screenshot marking where to place suggests combining it with the pool name as you have done

image

But the acceptance criteria says separate column

image

@brindasasi
Copy link
Contributor

Little confused here, which is it supposed to be?

The issue screenshot marking where to place suggests combining it with the pool name as you have done

image

But the acceptance criteria says separate column

image

This is a separate column

@yonikid15
Copy link
Contributor Author

@brindasasi Gray said it's supposed to be a separate column. Should have it finished tomorrow for another review.

@petertgiles petertgiles self-requested a review June 27, 2024 14:39
petertgiles
petertgiles previously approved these changes Jun 27, 2024
Copy link
Contributor

@petertgiles petertgiles left a comment

Choose a reason for hiding this comment

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

Nice!

@petertgiles petertgiles dismissed their stale review June 27, 2024 14:53

Sorry, missed the discussion about separate columns.

Copy link
Contributor

@vd1992 vd1992 left a comment

Choose a reason for hiding this comment

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

Took a look
One insignificant typo, one question, and one thing to definitely check on

api/app/Models/PoolCandidate.php Outdated Show resolved Hide resolved
@yonikid15
Copy link
Contributor Author

yonikid15 commented Jun 28, 2024

@vd1992 @brindasasi @petertgiles Ready for a review now 😆 whoever wants to jump on it first

Oh nvm Vachan got to it!

@yonikid15 yonikid15 requested a review from vd1992 June 28, 2024 15:07
Copy link
Contributor

@vd1992 vd1992 left a comment

Choose a reason for hiding this comment

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

Nice, looks great!

@yonikid15 yonikid15 added this pull request to the merge queue Jun 28, 2024
Merged via the queue into main with commit f45f1ea Jun 28, 2024
11 of 12 checks passed
@yonikid15 yonikid15 deleted the 10695-add-process-number-to-tables branch June 28, 2024 17:39
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.

✨ Process number needs to be added into additional places
5 participants