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

cleanup compensation and add the start of bonus #569

Merged
merged 10 commits into from
Sep 4, 2024

Conversation

christinaroise
Copy link
Contributor

@christinaroise christinaroise commented Sep 3, 2024

While adding the compensationDetails to the existing compensation document, I noticed that our initial architecture was a little flawed. In the original structure, which was "sorted" buy location, we assigned individual titles, slugs, SEO fields, and compensation data (such as bonus, salary, etc.) to the location(s) set in the document. But, on our frontend we don't want different page titles, SEO data or slugs. we only want to show/hide content by location it is meant for. (I'm thinking a TAB-solution)

So in other words our initial setup was a little too overly complex, maybe even redundant, and would most likely lead to inefficient data management.

Solution
So, to address this, I restructured the compensation schema to reflect a more streamlined approach. Instead of assigning each location its own document structure, I updated the compensation document to have a unified structure with shared fields at the top level and location-specific compensation data grouped logically.

The key changes include:

  • deskStructure.ts: The compensation document now uses an object ({}) structure rather than an array ([]).
  • compensation.ts:
    -- Shared fields: title, slug, seo, and other core fields like pension are now shared across all locations.
    -- Location-Specific Fields: Fields that differ by location, such as bonusesByLocation, benefitsByLocation, and salaryByLocation, are clearly separated and scoped to handle location-based differences effectively.

‼️ bonusesByLocation is the only one actually added in this PR, I have added comments for future tasks so it's easy to see whats to be added where

‼️ bonusesByLocation is also ONLY been added to show the schema structure with location sorting. I have NOT taken the schema type of average bonus into account other than setting it to number. Whatever else it needs to be can be updated

‼️ THIS PR IS NOT ADDING ADJUSTMENT TO THE FRONTEND. That needs to be added as well to prevent crashes. But I want to make sure we're agreeing with this approach first.

‼️ THIS PR DOES INTRODUCE DATA CONFLICTS WITH PRODUCTION AND STAGING. We can write a script or just manually delete data added to staging before we merge. Since we're not actually working with PROD, I don't mind manual work for now.

Updated Compensation Structure
The new structure consists of:
• title: Main title for the compensation page
• seo: SEO metadata shared across all locations
• slug: URL slug for the compensation page
• pension: Global pension details
• bonusesByLocation: Location-specific bonuses
• benefitsByLocation: Location-specific benefits
• salaryByLocation: Location-specific salary details

// moved seo to the bottom, I want to check with Linn-Helen, but I think it makes more sense to add SEO at the bottom once all the content is added and it's clear what title, description, and keywords should consist of.

Screenshot 2024-09-04 at 14 21 23 Screenshot 2024-09-04 at 14 21 34

ERROR state for duplicated locations. An idea is to create a custom component for Bonus. Ideally I'd like for Bonus to dynamically show an average amount field for all locations available, and error states for locations missing an average bonus amount
Screenshot 2024-09-04 at 14 21 59

UPDATE for Company Details -> Company Locations
Screenshot 2024-09-04 at 14 25 36

Copy link

vercel bot commented Sep 3, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
variant-no ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 4, 2024 1:16pm
variant-se-2 ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 4, 2024 1:16pm

@christinaroise christinaroise changed the title cleanup compensation and add compensationDetails cleanup compensation and add the start of bonus Sep 4, 2024
@@ -12,7 +12,7 @@ import posts from "./schemas/documents/post";
import categories from "./schemas/fields/categories";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

for a better differentiation between i.e benefits.locations and companyLocations I renamed this part

}),
// add pension here. pension doesn't rely on locations
bonusesByLocation,
// add salary here
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want to keep these comments so we know what's expected here once we add pension, benefits and salary

};
},
},
});

/**
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we most likely want to copy paste this into benefits and salary but removing for now.

@anemne
Copy link
Contributor

anemne commented Sep 4, 2024

I agree with this new structure! One question: Shouldn't the pension field also have an office location, or should we add it later if necessary?

@christinaroise
Copy link
Contributor Author

I agree with this new structure! One question: Shouldn't the pension field also have an office location, or should we add it later if necessary?

If I remember, Sweden and Norway both said that pension will be the same for all locations

@mathiazom mathiazom merged commit d6165da into v3 Sep 4, 2024
6 checks passed
@mathiazom mathiazom deleted the cleanup-compensation-and-add-compensationDetaisl branch September 4, 2024 13:16
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.

3 participants