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

Only calculate penalty reg time if heedPenalties #3732

Merged
merged 1 commit into from Mar 31, 2023

Conversation

LudvigHz
Copy link
Member

@LudvigHz LudvigHz commented Mar 30, 2023

Description

We don't check if the event uses penalties before calculating the
activation time. Also the calculation subtracted hours instead of adding
for some reason.

Also fixes random type errors in the mix here.

Result

The correct time is shown

Testing

  • I have thoroughly tested my changes. I have not tested anything :)). Probably flawless ™️.

Resolves ABA-356

@linear
Copy link

linear bot commented Mar 30, 2023

ABA-356 Event activation time is incorrect when applying penalties

Several issues:

  • activation time is shown with regards to penalties even when the event does not use penalties.
  • The penalty activation time is incorrect and subtracts hours instead of adds???

@github-actions github-actions bot added the review-needed Pull requests that need review label Mar 30, 2023
Copy link
Member

@ivarnakken ivarnakken left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for cleaning up the other stuff as well🎖️

@@ -477,7 +476,6 @@ const JoinEventForm = (props: Props) => {
component={TextInput.Field}
label={feedbackLabel}
className={styles.feedbackText}
fieldClassName={styles.feedbackField}
Copy link
Member

Choose a reason for hiding this comment

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

Did the css class not exist, or did you forget to remove it? Or is it used anywhere else? I'm on my phone so I can't check sorry

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah it doesn't exist. TS gives you an error when you use styles that don't exist :))

@ivarnakken ivarnakken added types Pull requests that improve or fix types bug-fix Pull requests that fix a bug approved Pull requests that have been approved technical-debt Pull requests that reduces technical debt and removed review-needed Pull requests that need review labels Mar 31, 2023
@ivarnakken
Copy link
Member

I have thoroughly tested my changes. I have not tested anything :)). Probably flawless ™️.

💀

We don't check if the event uses penalties before calculating the
activation time.

Also fixes random type errors in the mix here.
@github-actions github-actions bot added the review-needed Pull requests that need review label Mar 31, 2023
@LudvigHz
Copy link
Member Author

I have now tested the changes, and it was not flawless 🙃.

I realized the subtraction is to "remove" the penalty hours from LEGO, so we display the actual activation time. I guess this might be more clear as well. Why we're doing it in the frontend I don't know.

@LudvigHz LudvigHz merged commit 4d977d8 into master Mar 31, 2023
2 checks passed
@LudvigHz LudvigHz deleted the penalty-activation-time branch March 31, 2023 11:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Pull requests that have been approved bug-fix Pull requests that fix a bug review-needed Pull requests that need review technical-debt Pull requests that reduces technical debt types Pull requests that improve or fix types
Projects
None yet
2 participants