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: Optional floor area #288

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

Conversation

gabrielegranello
Copy link
Collaborator

What does this PR do?

Following #270 , it allows houseSize in the input to be undefined, so that it becomes an optional parameter. If undefined, the value is then mapped to the number of bedrooms.

#How?
The value is made optional in the formSchema and calculationSchema. Note that houseSize is not necessary to fetch the data in the API. Later, when the Property class is actually constructed, there is an if condition that checks if houseSize exists or needs to be mapped.

#Anything else?
We used to have a file named testClasses.ts but it was not a test. The main function within it was called calculateFairhold. I think the name was confusing, so I renamed it calculateFairhold.ts. Last, I added a series on unit tests in calculateFairhold.test.ts

Copy link

vercel bot commented Jan 17, 2025

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

Name Status Preview Comments Updated (UTC)
fairhold-dashboard ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 17, 2025 4:46pm

@gabrielegranello gabrielegranello marked this pull request as ready for review January 17, 2025 16:47
@gabrielegranello gabrielegranello requested a review from a team January 17, 2025 16:47
@@ -35,13 +35,27 @@ function calculateFairhold(responseData: ResponseData) {
throw new Error("maintenancePercentage data is missing or empty");
}

function assignHouseSize(numberOfBedrooms: number) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd view this functionality as part of our data validation, which is being handled by zod.

It's acceptable for the API to receive houseSize as undefined, but if it does it should map to a number based on the supplied number of rooms.

This approach would maintain the separation between data validation and business logic.

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