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

change recruitment status and priority to be fetched from backend #1604

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

magsyg
Copy link
Contributor

@magsyg magsyg commented Nov 5, 2024

closes #1575

@magsyg magsyg self-assigned this Nov 5, 2024
Snorre98
Snorre98 previously approved these changes Nov 7, 2024
Copy link
Contributor

@Snorre98 Snorre98 left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines 49 to 51
//recruiter_priority: [number, string][];
//recruiter_status: [number, string][];
getRecruitmentApplicationStateChoices().then((response) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
//recruiter_priority: [number, string][];
//recruiter_status: [number, string][];
getRecruitmentApplicationStateChoices().then((response) => {
getRecruitmentApplicationStateChoices().then((response) => {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@@ -23,21 +24,6 @@ type RecruitmentApplicantsStatusProps = {
onInterviewChange: () => void;
};

// TODO add backend to fetch these
const priorityOptions: DropdownOption<number>[] = [
Copy link
Member

Choose a reason for hiding this comment

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

Uenig i at dette er en god løsning. Vi kommer sannsynligvis aldri til å endre recruitment status options, så da er det fullstendig unødvendig å fetche fra backend bare for å populate en dropdown. Det blir ekstremt mange unødvendige api-kall. Mye bedre å bare la det være hardkodet i frontend.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True, men er mer for sikkerhet at inputtene i frontend og backend matcher.
Er mer sikkert når vi har siden, siden de alternativene alltid vil være like
.

Copy link
Contributor

@Snorre98 Snorre98 left a comment

Choose a reason for hiding this comment

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

robin mener dette er data vi ikke trenger å hente

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.

Fetch priority options and status options for application
3 participants