Skip to content

fix: day-week-in-month-date-time-between-failures#12809

Open
hemantmm wants to merge 19 commits intokestra-io:developfrom
hemantmm:day-week-in-month-date-time-between-failures
Open

fix: day-week-in-month-date-time-between-failures#12809
hemantmm wants to merge 19 commits intokestra-io:developfrom
hemantmm:day-week-in-month-date-time-between-failures

Conversation

@hemantmm
Copy link
Contributor

@hemantmm hemantmm commented Nov 8, 2025

closes: #8230

What changes are being made and why?

✅ Fixed dynamic property rendering for date in DayWeekInMonth and DateTimeBetween conditions.
✅ Updated usage to .render(date).as(String.class, vars).orElseThrow() for correct value extraction.
✅ Ensured tests pass after migration from String date to Property<String> date.


Fix:

Updated the rendering logic for dynamic date properties in schedule conditions:

  • Migrated date field from String to Property<String> in both classes.
  • Changed rendering from runContext.render(date, vars) to runContext.render(date).as(String.class, vars).orElseThrow().

@github-project-automation github-project-automation bot moved this to To review in Pull Requests Nov 8, 2025
@MilosPaunovic MilosPaunovic added kind/external Pull requests raised by community contributors area/backend Needs backend code changes labels Nov 10, 2025
@hemantmm
Copy link
Contributor Author

@loicmathieu, can you review this PR?

@hemantmm
Copy link
Contributor Author

hemantmm commented Nov 14, 2025

Previously, it was failing in the old PR, but now all the schedule tests passed.
image

@hemantmm hemantmm requested a review from loicmathieu November 18, 2025 14:01
@loicmathieu loicmathieu mentioned this pull request Nov 19, 2025
@fhussonnois
Copy link
Member

fhussonnois commented Nov 19, 2025

Hey @hemantmm, thank you very much for you contribution. Could you rebase your PR with the develop as some fixed have been made on the Schedule and DateTimeBetween classes.

Also, could you please reevaluate the changes you've made on the Schedule#truePreviousNextDateWithCondition method. In fact, we found that for the DateTimeBetween the rendering of the date was cached leading to wrong evaluation of the condition.

Thanks a lot.

Signed-off-by: Hemant M Mehta <hemant29mehta@gmail.com>
Signed-off-by: Hemant M Mehta <hemant29mehta@gmail.com>
Signed-off-by: Hemant M Mehta <hemant29mehta@gmail.com>
Signed-off-by: Hemant M Mehta <hemant29mehta@gmail.com>
…hen conditions fail

Signed-off-by: Hemant M Mehta <hemant29mehta@gmail.com>
Signed-off-by: Hemant M Mehta <hemant29mehta@gmail.com>
Signed-off-by: Hemant M Mehta <hemant29mehta@gmail.com>
Signed-off-by: Hemant M Mehta <hemant29mehta@gmail.com>
Signed-off-by: Hemant M Mehta <hemant29mehta@gmail.com>
Signed-off-by: Hemant M Mehta <hemant29mehta@gmail.com>
@hemantmm hemantmm force-pushed the day-week-in-month-date-time-between-failures branch from 8751683 to 0a6aa73 Compare November 20, 2025 12:31
Signed-off-by: Hemant M Mehta <hemant29mehta@gmail.com>
@hemantmm
Copy link
Contributor Author

@loicmathieu, can you review this PR?

@loicmathieu loicmathieu requested review from fhussonnois and removed request for loicmathieu November 25, 2025 09:17
@loicmathieu
Copy link
Member

@hemantmm @fhussonnois will review it as he worked on this area lately

@hemantmm
Copy link
Contributor Author

@fhussonnois, can you review this PR?

Signed-off-by: Hemant M Mehta <hemant29mehta@gmail.com>
@hemantmm hemantmm requested a review from Skraye December 1, 2025 16:39
@hemantmm
Copy link
Contributor Author

hemantmm commented Dec 4, 2025

@fhussonnois, can you review this PR?

@loicmathieu
Copy link
Member

@Skraye will continue the review when he'll be back

@hemantmm
Copy link
Contributor Author

@Skraye, can you review this PR?

@MilosPaunovic
Copy link
Member

@hemantmm Please resolve the merge conflict here.

@MilosPaunovic MilosPaunovic moved this from To review to On hold in Pull Requests Mar 4, 2026
@Skraye
Copy link
Member

Skraye commented Mar 4, 2026

@hemantmm Can you make sure the project compile ? Thanks

hemantmm and others added 3 commits March 6, 2026 18:40
@hemantmm
Copy link
Contributor Author

hemantmm commented Mar 6, 2026

@Skraye, now the project compiles. Can you have a look at it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/backend Needs backend code changes kind/external Pull requests raised by community contributors

Projects

Status: On hold

Development

Successfully merging this pull request may close these issues.

Dynamic properties fail tests for DayWeekInMonth and DateTimeBetween

5 participants