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] Opt into search placed candidates backend #12330

Merged
merged 11 commits into from
Dec 19, 2024

Conversation

vd1992
Copy link
Contributor

@vd1992 vd1992 commented Dec 17, 2024

πŸ€– Resolves #12212

πŸ‘‹ Introduction

Handles some of the backend work needed as well as introduces an Artisan command.

πŸ•΅οΈ Details

The command will update placed term and indeterminate candidates to have suspended dates.
Placement mutations will touch the suspended date field, and search accounts for the two statuses as well.

Issue to delete command #12331

πŸ§ͺ Testing

  1. Test Artisan command
  2. Place/revert candidates, assert field updated in database client
  3. Test placed candidate searchable or not based off suspended value, such as from /en/search

🚚 Deployment

Run
php artisan app:suspend-placed-candidates

Comment on lines +832 to +833
// asserts placed term/indeterminate candidates can appear in this query, can't check by id though so check that 2 out of 3 appear
public function testPlacedSuspendedNotSuspendedCandidates()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Supplemented by testScopeAvailable()

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.

Really nice work. πŸ‘ Mostly, I'm just asking for more comments. πŸ˜† A few things to check out but this is great work.

api/app/Console/Commands/SuspendPlacedCandidates.php Outdated Show resolved Hide resolved
api/app/Console/Commands/SuspendPlacedCandidates.php Outdated Show resolved Hide resolved
api/app/GraphQL/Mutations/PlaceCandidate.php Show resolved Hide resolved
api/tests/Feature/CountPoolCandidatesByPoolTest.php Outdated Show resolved Hide resolved
api/tests/Feature/PoolCandidateUpdateTest.php Outdated Show resolved Hide resolved
api/tests/Feature/UserTest.php Show resolved Hide resolved
@vd1992 vd1992 requested a review from petertgiles December 18, 2024 21:30
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.

Very nice!

@petertgiles petertgiles added the deployment Requires a change during deployment label Dec 19, 2024
@vd1992
Copy link
Contributor Author

vd1992 commented Dec 19, 2024

@petertgiles

Appears to populate the activity log. All good to go then?

image

@petertgiles
Copy link
Contributor

Make it so.

@petertgiles
Copy link
Contributor

Could you please also try running the Eloquent command in Tinker, locally?

@vd1992
Copy link
Contributor Author

vd1992 commented Dec 19, 2024

Could you please also try running the Eloquent command in Tinker, locally?

It's a little tricky

First, the spacing/newlines can mess it up so you have to adjust it
Second, it aliases a typing incorrectly

image

@petertgiles
Copy link
Contributor

It's a little tricky

Ah, too bad. Are you able to provide the commands that we can copy/paste for prod?

@vd1992
Copy link
Contributor Author

vd1992 commented Dec 19, 2024

It's a little tricky

Ah, too bad. Are you able to provide the commands that we can copy/paste for prod?

Going back to the facade seems to do the trick

$applicableStatuses = [PoolCandidateStatus::PLACED_TERM->name, PoolCandidateStatus::PLACED_INDETERMINATE->name];

DB::table('pool_candidates')->whereIn('pool_candidate_status', $applicableStatuses)->whereNull('suspended_at')->update(['suspended_at' => Carbon\Carbon::now()]);

image

image

@petertgiles
Copy link
Contributor

I dont' think it's necessary to go back to the DB facade. You can use use commands in your Tinker environment to avoid the aliasing problems and backslashes to add multiline commands. Here's what worked for me.

use App\Enums\PoolCandidateStatus;
use App\Models\PoolCandidate;
use Illuminate\Database\Eloquent\Collection;
use Carbon\Carbon;

$applicableStatuses = [PoolCandidateStatus::PLACED_TERM->name, PoolCandidateStatus::PLACED_INDETERMINATE->name];

PoolCandidate::whereIn('pool_candidate_status', $applicableStatuses) \
    ->whereNull('suspended_at') \
    ->with('user') \
    ->chunkById(100, function (Collection $candidates) { \
        foreach ($candidates as $candidate) { \
            $candidate->suspended_at = Carbon::now(); \
            $candidate->save(); \
        } \
    } \
);

image

@vd1992
Copy link
Contributor Author

vd1992 commented Dec 19, 2024

I guess we've got what we need then

@vd1992 vd1992 added this pull request to the merge queue Dec 19, 2024
Merged via the queue into main with commit c776307 Dec 19, 2024
7 checks passed
@vd1992 vd1992 deleted the 12212-opt-into-search-placed-candidates-backend branch December 19, 2024 19:45
@brindasasi
Copy link
Contributor

php artisan app:suspend-placed-candidates
Doesn't work in Dev.
image (37)
copy pasting those tinker scripts worked well .. except those deprecated warnings .. I guess it is still no harm.
image (38)

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

Successfully merging this pull request may close these issues.

✨ Allow users to opt back into talent searches after they are hired
3 participants