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] Bookmark pool candidates #8767

Merged
merged 18 commits into from
Jan 5, 2024
Merged

Conversation

JamesHuf
Copy link
Contributor

🤖 Resolves #8598

👋 Introduction

Adds the ability to bookmark pool candidates in the assessment step tracker.

🧪 Testing

The assessment step tracker is currently not used on any page so testing must be done in storybook.

  1. Manually test the new mutation and confirm it works as expected
mutation ToggleBookmark_Mutation($id: ID!) {
    togglePoolCandidateBookmark(id: $id)
}
  1. Open the assessment step tracker story
  2. Confirm already bookmarked candidates are displayed at the top of their columns.
  3. Click to bookmark a candidate and confirm they are NOT immediately re-sorted to the top of the column.

📸 Screenshot

image

…s.tsx


Add apostrophe to translation

Co-authored-by: Matt <[email protected]>
@petertgiles petertgiles self-requested a review December 14, 2023 13:34
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.

Cool, it's looking great! 🌟 I've noted a few things to check on.

api/graphql/schema.graphql Show resolved Hide resolved
apps/web/src/components/AssessmentStepTracker/utils.ts Outdated Show resolved Hide resolved
api/graphql/schema.graphql Outdated Show resolved Hide resolved
@petertgiles
Copy link
Contributor

Had a quick review:

  1. Comments explaining step-by-step in sort function.
  2. Possible refactoring in sort function.
  3. Magic number in Jest test.
  4. Small unit test for just sort function in Jest testing.

@JamesHuf
Copy link
Contributor Author

Had a quick review:

  1. Comments explaining step-by-step in sort function.
  2. Possible refactoring in sort function.
  3. Magic number in Jest test.
  4. Small unit test for just sort function in Jest testing.

I've made the changes mentioned as well as adding sorting by name since I realized I'd missed that. I'm not a huge fan of the new test though since it feels kind of hard to follow.

@JamesHuf
Copy link
Contributor Author

Also @Jerryescandon are active members of the armed forces considered veterans in terms of priority/ordering? It looks like we treat them the same as non members right now and I'd like to make sure that's right.

@petertgiles
Copy link
Contributor

I've made the changes mentioned as well as adding sorting by name since I realized I'd missed that. I'm not a huge fan of the new test though since it feels kind of hard to follow.

One possibility is that you can make the test more expressive by adding descriptive IDs to each element instead of just integers:
image
This makes the assertion part of the test a bit clearer:
image

@petertgiles petertgiles added the blocked: copy Blocked by missing copy or translations label Dec 19, 2023
@Jerryescandon
Copy link

Also @Jerryescandon are active members of the armed forces considered veterans in terms of priority/ordering? It looks like we treat them the same as non members right now and I'd like to make sure that's right.

That sounds correct, I believe only the third option should trigger a priority change:
image

@JamesHuf
Copy link
Contributor Author

Awesome! Thank you!

@JamesHuf JamesHuf added this pull request to the merge queue Jan 5, 2024
Merged via the queue into main with commit fab7f82 Jan 5, 2024
10 checks passed
@JamesHuf JamesHuf deleted the 8598-bookmark-pool-candidates branch January 5, 2024 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked: copy Blocked by missing copy or translations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

✨ Bookmark a pool candidate
5 participants