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

Prevent bookings with same start / stop time #3304

Merged
merged 15 commits into from May 17, 2022
Merged

Prevent bookings with same start / stop time #3304

merged 15 commits into from May 17, 2022

Conversation

Wolfimschafspelz
Copy link
Contributor

@Wolfimschafspelz Wolfimschafspelz commented May 11, 2022

Description

  • Makes it impossible to create bookings with the same start / stop time when unchecking a box in the system settings

grafik

grafik

Types of changes

  • New feature (non-breaking change which adds functionality

Checklist

  • I verified that my code applies to the guidelines (composer code-check)
  • I agree that this code is used in Kimai and will be published under the MIT license

Copy link
Member

@kevinpapst kevinpapst left a comment

Choose a reason for hiding this comment

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

Thank @Wolfimschafspelz 👍

We need to add some missing translations, so they show up in Weblate for the translation to all other languages.

In validators.en.xlf the key Duration cannot be zero. with the same translations.

And your username suggests that you can add the German translations for the two new messages 😄 ?!?

src/Validator/Constraints/TimesheetZeroLengthValidator.php Outdated Show resolved Hide resolved
src/Validator/Constraints/TimesheetZeroLength.php Outdated Show resolved Hide resolved
src/Validator/Constraints/TimesheetZeroLength.php Outdated Show resolved Hide resolved
Copy link
Member

@kevinpapst kevinpapst left a comment

Choose a reason for hiding this comment

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

Hey, after giving another thought, I'd like to see two more changes:

  • replace "Length" with "Duration" in all places
  • add a unit test for the validator

Edit: the last fixes were exactly what I had in mind, thank you 👍

@kevinpapst kevinpapst added this to the 1.20 milestone May 12, 2022
Copy link
Member

@kevinpapst kevinpapst left a comment

Choose a reason for hiding this comment

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

There are some other issues in existing tests, due to the added configuration. I can solve them later on, if you don't find the solution.

@kevinpapst
Copy link
Member

Please run composer codestyle-fix and composer phpstan before committing.

@codecov
Copy link

codecov bot commented May 15, 2022

Codecov Report

Merging #3304 (2ca2d47) into master (68cd879) will increase coverage by 0.03%.
The diff coverage is 92.30%.

@@             Coverage Diff              @@
##             master    #3304      +/-   ##
============================================
+ Coverage     91.96%   91.99%   +0.03%     
- Complexity     7723     7732       +9     
============================================
  Files           736      738       +2     
  Lines         23312    23338      +26     
============================================
+ Hits          21438    21470      +32     
+ Misses         1874     1868       -6     
Impacted Files Coverage Δ
...rc/Validator/Constraints/TimesheetZeroDuration.php 0.00% <0.00%> (ø)
src/Configuration/SystemConfiguration.php 98.69% <100.00%> (+0.01%) ⬆️
src/Controller/SystemConfigurationController.php 98.16% <100.00%> (+0.01%) ⬆️
src/DependencyInjection/Configuration.php 97.70% <100.00%> (+<0.01%) ⬆️
...tor/Constraints/TimesheetZeroDurationValidator.php 100.00% <100.00%> (ø)
src/Timesheet/DateTimeFactory.php 96.25% <0.00%> (-1.25%) ⬇️
src/Controller/QuickEntryController.php 75.67% <0.00%> (+8.10%) ⬆️

Copy link
Member

@kevinpapst kevinpapst left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

Thank you @Wolfimschafspelz for being patient 😄

@kevinpapst kevinpapst merged commit 27c60d9 into kimai:master May 17, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Nov 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

Prevent bookings with same start / stop time
2 participants