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

feat: interview reviews base 🧱 #399

Conversation

tomas-salgado
Copy link
Collaborator

@tomas-salgado tomas-salgado commented Jul 22, 2024

Description ✏️

This PR implements the first part of the interview reviews feature.
It adds

  • interview_reviews table to the database
  • Fetch and list all interview reviews for a given company
  • Implementation of navigation between Employees, Company Reviews, and Interview Reviews on Companies page
  • Put front-end changes in _profile.companies_.$id.tsx behind a new feature flag named interview-reviews

Type of Change 🐞

  • Feature - A non-breaking change which adds functionality.
  • Fix - A non-breaking change which fixes an issue.
  • Refactor - A change that neither fixes a bug nor adds a feature.
  • Documentation - A change only to in-code or markdown documentation.
  • Tests - A change that adds missing unit/integration tests.
  • Chore - A change that is likely none of the above.

Checklist ✅

  • I have done a self-review of my code.
  • I have manually tested my code (if applicable).
  • I have added/updated any relevant documentation (if applicable).

@tomas-salgado
Copy link
Collaborator Author

tomas-salgado commented Jul 22, 2024

I decided to break up the feature into multiple PRs-it was getting too messy and this makes more sense. I'm curious if I should create stacked PRs and then not merge any until all the PRs are approved, or if I should just put the interview reviews feature behind a feature flag, so that way we can merge them in one at a time but it won't appear to members until the entire feature is complete.

UPDATE: I went with putting the front-end changes behind a feature flag interview-reviews.

I also have a PR up for the next part (feat: add interview reviews). It's in my personal fork because it's stacked on top of this branch, but once this is merged I'll be able to move the PR to the main repo here.

@@ -4,6 +4,7 @@ import {
type SerializeFrom,
} from '@remix-run/node';
import { generatePath, Link, useLoaderData } from '@remix-run/react';
import { useSearchParams } from '@remix-run/react';
Copy link
Collaborator Author

@tomas-salgado tomas-salgado Jul 22, 2024

Choose a reason for hiding this comment

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

It seems there is also useSearchParams from @oyster/ui here. Curious if this should be used here?

@tomas-salgado tomas-salgado marked this pull request as ready for review July 26, 2024 19:28
@tomas-salgado
Copy link
Collaborator Author

One minor issue I was facing is that the navigation tabs here always show up green. This is different from the recaps page, for example, where the tab is only green if it is selected. Having trouble figuring out the issue here.

Screenshot 2024-07-29 at 2 47 10 PM

@ramiAbdou ramiAbdou marked this pull request as draft August 29, 2024 17:06
@ramiAbdou
Copy link
Member

Going to close for now, and we'll revisit this in the future!

@ramiAbdou ramiAbdou closed this Sep 23, 2024
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.

2 participants