[59539] Add automatic scheduling mode #17235
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cbliard
force-pushed
the
feature/42388-new-automatic-scheduling-mode
branch
3 times, most recently
from
46a22d1 to
391d65b
Compare
cbliard
changed the title
cbliard
force-pushed
the
feature/42388-new-automatic-scheduling-mode
branch
4 times, most recently
from
5f715b3 to
13755a0
Compare
cbliard
force-pushed
the
feature/42388-new-automatic-scheduling-mode
branch
from
ec01bc0 to
7923850
Compare
cbliard
force-pushed
the
feature/42388-new-automatic-scheduling-mode
branch
3 times, most recently
from
e256a4f to
a90fb94
Compare
cbliard
force-pushed
the
feature/42388-new-automatic-scheduling-mode
branch
from
c5e476e to
0fb2ae7
Compare
cbliard
changed the title
cbliard
force-pushed
the
feature/42388-new-automatic-scheduling-mode
branch
5 times, most recently
from
be26132 to
ee699b6
Compare
cbliard
force-pushed
the
feature/42388-new-automatic-scheduling-mode
branch
from
ee699b6 to
0d14a71
Compare
cbliard
force-pushed
the
feature/42388-new-automatic-scheduling-mode
branch
2 times, most recently
from
202abab to
3fff7d3
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| scope.pluck(:id).each do |id| | ||
| next if already_processed_work_package_ids.include?(id) | ||
|
|
||
| processed_work_packages = yield(WorkPackage.find(id)) |
Check notice
Code scanning / CodeQL
Database query in a loop Note
The date picker preview controller and the progress preview controller were abstracted into a super class. It's too hard to make deep modifications to the date picker preview controller while keeping the compatibility with the progress preview controller. For instance, we break the progress values being formatted on blur without noticing. This commit adds a test just for this. This commit undoes some changes made in 5429b8b (commit title is "Extract logic of the progress modal to be reused for the datepicker modal"). Once the date picker progress controller works as expected, we'll see which part can be reused and extracted into an abstraction.
This logic was in the date picker component, meaning it was triggered when picking another date, but not when editing dates in the input fields directly. This commit moves the logic to the preview controller, and removes almost all responsibilities from the date picker component: - It sends `date-picker:flatpickr-dates-changed` events to the controller when new dates are picked - It receives `date-picker:flatpickr-set-values events from the controller to update its dates, mode, and display of non-working days. When the update is done, it does it without reinitializing itself. Some logic still remains in the date picker component for its initialization which is based on its HTML attributes. The controller also changed how it gets the dates values after a turbo morphing: the values are read from HTML inputs once the morph rendering is finished. There are also 2 different debounce for the preview: one of 200ms when the user is typing in the input fields, and one of 0ms when the user is clicking on a date in the calendar. In that case the debounce is not needed as the date is set immediately.
If the dates have not changed, the date picker is not reinitialized and its dates are not set again. So switching "Working-days only" checkbox no longer shows the current date again and it's not needed for the test to wait for the current date to be displayed and switch back to the correct date.
And fix rubocop Metrics/AbcSize offense.
…r-automatic-scheduling-mode [59845] Update Datepicker for automatic scheduling mode
The parent dates are calculated based on the earliest and latest dates of its children.
There was a bug where a parent of a work package switching to automatic would not be properly rescheduled.
If that works, that means the schedule service can be called also for isolated work packages without predecessors nor children, and the service could update the scheduling mode of such work packages.
When a work package no longer have any predecessors (direct or indirect) nor children, then it must switch to manual scheduling mode to keep its dates. This is now handled directly by the `WorkPackages::SetScheduleService`. This simplifies the logic of `Relations::DeleteService` which no longer has to switch a successor to manual by itself. It's all handled in the ScheduleService.
A work package can be created with `schedule_manually: false` only if it has children or predecessors. With the API it's not possible to create a work package with children directly. But it's possible to create a work package with a parent with a predecessor. In this case, it's possible to set scheduling mode to automatic. This commit adds a new spec to cover this case.
There was a bug were when considering the indirect predecessors of a work package, it would consider all ancestors. This was wrong: only the automatically scheduled ancestors should be considered, and ancestors after the first manually scheduled ancestor should be ignored.
from 19 secs to 10 secs.
From the deleted work package, its parent, its successors, and the successors of its descendants are rescheduled and switched to manual scheduling if appropriate.
cbliard
force-pushed
the
feature/42388-new-automatic-scheduling-mode
branch
from
5e9d0a4 to
1517b7c
Compare
There are more important things to do to ensure this can be merged before 15.4 freeze. In the meantime, we'll test these scenarios manually and implement them later.
Comparing nil and dates does not work, so to sort due dates, nil values must be filtered out first. Also fixes another issue where the banner for overlapping predecessors was not displayed if a predecessor had no due date, due to early return in loop.
https://community.openproject.org/wp/61535 A work package can have ancestors having predecessors. These predecessors are indirect predecessors of the work package and are involved in the scheduling if all the ancestors up to the predecessor are automatically scheduled. When such an indirect predecessor is present, the work package can be switched to manual scheduling, even if it has no direct predecessors or children. It uses the scope `Relation#used_for_scheduling_of(work_package)` to find all predecessors involved in scheduling. This encompass both direct and indirect predecessors.
|
Both failing tests are passing locally. Merging. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ticket
https://community.openproject.org/wp/59539
(Part of https://community.openproject.org/wp/42388)
Pull preview
Available here: https://pr-17235-42388-new-autom-ip-3-70-132-70.my.preview.run/
What works:
What's next:
What's missing:
What are you trying to accomplish?
Implement new automatic scheduling mode
Migration:
Logic changes:
Feature implementation:
spec/services/work_packages/set_schedule_service_working_days_spec.rb:374)Screenshots
TBD
What approach did you choose and why?
TBD
Merge checklist