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

Jump to bottom

Sector Nord AG: Fixed bug - hour 0 is not possible in the default settings for TimeWorkingHours. #296

Merged
merged 2 commits into from Sep 29, 2022
Merged

Sector Nord AG: Fixed bug - hour 0 is not possible in the default settings for TimeWorkingHours. #296

merged 2 commits into from Sep 29, 2022

Conversation

LuBroering
Copy link
Contributor

@LuBroering LuBroering commented Sep 28, 2022 •
edited

Proposed change

This fixes a bug that caused the hour 0 to not be pre-selectable in the default settings.

How to reproduce the bug:

  1. change the working hours in Framework.xml (should be including a 0 as Value now)
    grafik
  2. Maint::Config::Rebuild
  3. Lookup the setting and see that 0 is not selected.

Type of change

  • '1 - 🐞 bug 🐞' - Bugfix (non-breaking change which fixes an issue)

Checklist

  • The code change is tested and works locally.(❗)
  • There is no commented out code in this PR.(❕)
  • You improved or added new unit tests.(❕)
  • Local ZnunyCodePolicy run passes successfully.(❕)
  • Local unit tests pass.(❕)
  • GitHub workflow ZnunyCodePolicy passes.(❗)
  • GitHub workflow unit tests pass.(❗)

@rkaldung rkaldung changed the title Sector Nord AG: fixed Bug were Hour 0 was not possible in Deault Settings for TimeWorkingHours Sector Nord AG: fixed Bug were Hour 0 was not possible in Default Settings for TimeWorkingHours Sep 28, 2022
@hanneshal hanneshal added 1 - 🐞 bug 🐞 An issue with the system. Znuny LTS Znuny LTS. 4 - verified This issue or pull request was verified. 1 - 💎 code quality Code quality improvements to existing code or addition of unit tests. Znuny Feature New Znuny version. labels Sep 28, 2022
@hanneshal
Copy link

I can verify this.
Thanks. good find

dennykorsukewitz
dennykorsukewitz reviewed Sep 28, 2022
View reviewed changes
Copy link
Member

@dennykorsukewitz dennykorsukewitz left a comment

Choose a reason for hiding this comment

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

Hi @LuBroering ,
thanks for your PR.

next HOUR if !defined $Hour->{Content};

A defined would have been the solution here. The regex is of course more precise. 👍🏼

Could you please add a comment briefly describing, that the regex allows the numbers from 0 to 23.

Regards 🚀

@dennykorsukewitz dennykorsukewitz added this to the rel-6_4_4 milestone Sep 28, 2022
@LuBroering
Copy link
Contributor Author

Hi @dennykorsukewitz,
I added the comment

@dennykorsukewitz dennykorsukewitz added the 3 - wait for pipeline Wait for green GitHub pipeline. label Sep 29, 2022
@dennykorsukewitz dennykorsukewitz removed the 3 - wait for pipeline Wait for green GitHub pipeline. label Sep 29, 2022
@dennykorsukewitz dennykorsukewitz changed the title Sector Nord AG: fixed Bug were Hour 0 was not possible in Default Settings for TimeWorkingHours Sector Nord AG: Fixed bug - hour 0 is not possible in the default settings for TimeWorkingHours. Sep 29, 2022
@dennykorsukewitz dennykorsukewitz merged commit f62366b into znuny:dev Sep 29, 2022
znuny-robo pushed a commit that referenced this pull request Oct 6, 2022
@LuBroering @dennykorsukewitz
Sector Nord AG: Fixed bug - hour 0 is not possible in the default set…
7e1d5fb 
…tings for TimeWorkingHours. (#296)

* fixed bug were hour 0 was not allowed
* added comment for the regex
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dennykorsukewitz dennykorsukewitz dennykorsukewitz approved these changes

Assignees

@dennykorsukewitz dennykorsukewitz

Labels
1 - 🐞 bug 🐞 An issue with the system. 1 - 💎 code quality Code quality improvements to existing code or addition of unit tests. 4 - verified This issue or pull request was verified. Znuny Feature New Znuny version. Znuny LTS Znuny LTS.
Milestone
rel-6_4_4
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@LuBroering @hanneshal @dennykorsukewitz