feat: Create StartDate and EndDate types [DHIS2-16019] #17454

enricocolasante · 2024-05-16T09:23:35Z

We are adding 2 new web parameter types for date filters StartDate and EndDate.
In a lot of APIs we have filters to get entities before and after something and most of them are of type DateTime.

Everything works correctly if the client specifies the time together with the date but if only the date is passed then the before filter default to the beginning of the day filtering out all the entities with the date in that day.

Now

StartDate passed without a time, defaults to the beginning of the day (some behavior already present in DateTime type)
EndDate passed without a time, defaults to the end of the day

StartDate and EndDate are meant to be used only in the web layer.

dhis-2/dhis-web-api/src/test/java/org/hisp/dhis/webapi/controller/DatesBindingTest.java

teleivo

StartDate and EndDate are meant to be used only in the web layer.

So these are used to convert the HTTP query parameters into a Date which is what we use in the service/repo layer?

I think it would make sense to already use these types in some controller in this PR.

dhis-2/dhis-api/src/main/java/org/hisp/dhis/util/DateUtils.java

teleivo · 2024-05-17T09:47:23Z

dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/webdomain/StartDate.java

+ *
+ * <p>This behavior, combined with {@link EndDate}, allows to correctly implement an interval search
+ * including start and end dates. startDate=2020-10-10&endDate=2020-10-12 will include anything
+ * between 2020-10-10T00:00:00.000 and 2020-10-12T23:59:59.999.


Should we leave

startDate=2020-10-10&endDate=2020-10-12 will include anything between 2020-10-10T00:00:00.000 and 2020-10-12T23:59:59.999.

from these docs here as this is at the discretion of the endpoint itself?

Or do we "standardize" on this behavior, in that case it makes sense to keep it here. I am not sure its 100% clear right now with include anything between that the endpoints are included as well. So maybe we could change the wording or add (represent a closed interval [startDate, endDate].

dhis-2/dhis-web-api/src/test/java/org/hisp/dhis/webapi/controller/DatesBindingTest.java

dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/CrudControllerAdvice.java

dhis-2/dhis-api/src/main/java/org/hisp/dhis/util/DateUtils.java

enricocolasante · 2024-05-20T11:56:26Z

StartDate and EndDate are meant to be used only in the web layer.

So these are used to convert the HTTP query parameters into a Date which is what we use in the service/repo layer?

I think it would make sense to already use these types in some controller in this PR.

I was trying to separate the creation of the parameter types and their application, but I agree that it makes sense to have at least one example.
I am not sure what is the best way to test them though

dhis-2/dhis-web-api/src/test/java/org/hisp/dhis/webapi/controller/DatesBindingTest.java

...i/src/main/java/org/hisp/dhis/webapi/controller/tracker/export/event/EventRequestParams.java

dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/webdomain/EndDate.java

dhis-2/dhis-web-api/src/test/java/org/hisp/dhis/webapi/controller/DatesBindingTest.java

teleivo · 2024-05-21T06:53:15Z

dhis-2/dhis-web-api/src/test/java/org/hisp/dhis/webapi/controller/SpringBindingTest.java

-        MockMvcBuilders.standaloneSetup(new BindingController())
-            .setControllerAdvice(new CrudControllerAdvice())
-            .build();
+    mockMvc = MockMvcBuilders.standaloneSetup(new BindingController()).build();


There is enum related code in the CrudControllerAdvice. Not sure what it does but I wonder if CrudControllerAdvice should stay in here or the code in CrudControllerAdvice relating enums should be removed if it has no influence. Either way maybe do this in a separate PR.

dhis-2/dhis-web-api/src/test/java/org/hisp/dhis/webapi/controller/DatesBindingTest.java

jbee

Happy to see this being introduced. As far as I could see from a glance this should work nicely. Some suggestions on API details.

...main/java/org/hisp/dhis/webapi/controller/tracker/export/event/EventRequestParamsMapper.java

jbee · 2024-05-21T20:13:16Z

dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/webdomain/EndDateTime.java

+public class EndDateTime {
+  private final Date date;
+
+  public static EndDateTime valueOf(String date) {


Javadoc on the list of valid patterns for date would be nice. Is this YYYYmmdd only?

jbee · 2024-05-21T20:15:44Z

dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/webdomain/StartDateTime.java

+public class StartDateTime {
+  private final Date date;
+
+  public static StartDateTime valueOf(String date) {


Not a strong opinion but in newer Java APIs this is usually just called of, e.g. List.of, LocalDate.of

of should work as well with Springs conversion system as it looks for either a constructor or a factory called of, valueOf or from.

(see #16673)

teleivo · 2024-05-22T10:41:38Z

dhis-2/dhis-web-api/src/test/java/org/hisp/dhis/webapi/controller/DatesBindingTest.java

+  @NoArgsConstructor
+  @Data
+  private class Params {
+    private StartDateTime after;


start and end would probably make it easier to read the test as one does not need to translate what before refers to.

sonarcloud · 2024-05-22T10:54:00Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

dhis-2/dhis-web-api/src/test/java/org/hisp/dhis/webapi/controller/DatesBindingTest.java

+
+  @NoArgsConstructor
+  @Data
+  private class Params {


dhis-2/dhis-web-api/src/test/java/org/hisp/dhis/webapi/controller/DatesBindingTest.java

+  }
+
+  @NoArgsConstructor
+  @Data


* feat: Create StartDate and EndDate types [DHIS2-16019] * Fix docs * Fix tests * Fix tests * Fix review comments * Fix review comments * Fix review comments * Fix review comments * Fix review comments * Fix review comments * Fix review comments * fix: Remove unused tracker services from program-rule module * Fix review comments * Fix review comments * Fix review comments

…9] (2.41) (#17646) * feat: Create StartDate and EndDate types [DHIS2-16019] (#17454) * feat: Create StartDate and EndDate types [DHIS2-16019] * Fix docs * Fix tests * Fix tests * Fix review comments * Fix review comments * Fix review comments * Fix review comments * Fix review comments * Fix review comments * Fix review comments * fix: Remove unused tracker services from program-rule module * Fix review comments * Fix review comments * Fix review comments * fix: Use new start and end date types in tracker exporter [DHIS2-16019] (#17522) * fix: Use new start and end date types in tracker exporter [DHIS2-16019] * Fix review comments * Fix review comments * docs: Update OpenAPI for tracker exporter endpoints [DHIS2-16019] (#17569) * docs: Update OpenAPI for tracker exporter endpoints [DHIS2-16019] * Fix review comments * Use last OpenApi changes

…9] (2.40) (#17648) * feat: Create StartDate and EndDate types [DHIS2-16019] (#17454) * fix: Use new start and end date types in tracker exporter [DHIS2-16019] (#17522) * Fix tests

…9] (2.39) (#17650) * feat: Create StartDate and EndDate types [DHIS2-16019] (#17454) * fix: Use new start and end date types in tracker exporter [DHIS2-16019] (#17522) * Fix test

feat: Create StartDate and EndDate types [DHIS2-16019]

cd650f1

github-advanced-security bot found potential problems May 16, 2024

View reviewed changes

enricocolasante added 3 commits May 16, 2024 11:59

Fix docs

beb04cf

Fix tests

c0fdefd

Fix tests

3185c25

github-advanced-security bot found potential problems May 16, 2024

View reviewed changes

dhis-2/dhis-web-api/src/test/java/org/hisp/dhis/webapi/controller/DatesBindingTest.java Fixed Show fixed Hide fixed

enricocolasante marked this pull request as ready for review May 16, 2024 14:37

enricocolasante requested a review from a team May 16, 2024 14:37

teleivo reviewed May 17, 2024

View reviewed changes

muilpp reviewed May 17, 2024

View reviewed changes

dhis-2/dhis-api/src/main/java/org/hisp/dhis/util/DateUtils.java Outdated Show resolved Hide resolved

enricocolasante added 2 commits May 20, 2024 13:44

Fix review comments

4937348

Fix review comments

9c2bbb8

github-advanced-security bot found potential problems May 20, 2024

View reviewed changes

dhis-2/dhis-web-api/src/test/java/org/hisp/dhis/webapi/controller/DatesBindingTest.java Fixed Show fixed Hide fixed

enricocolasante requested a review from muilpp May 20, 2024 12:11

muilpp approved these changes May 20, 2024

View reviewed changes

teleivo reviewed May 21, 2024

View reviewed changes

enricocolasante added 2 commits May 21, 2024 15:48

Fix review comments

0d97920

Fix review comments

e9c3a7c

github-advanced-security bot found potential problems May 21, 2024

View reviewed changes

dhis-2/dhis-web-api/src/test/java/org/hisp/dhis/webapi/controller/DatesBindingTest.java Fixed Show fixed Hide fixed

dhis-2/dhis-web-api/src/test/java/org/hisp/dhis/webapi/controller/DatesBindingTest.java Fixed Show fixed Hide fixed

enricocolasante requested a review from teleivo May 21, 2024 14:22

jbee approved these changes May 21, 2024

View reviewed changes

enricocolasante added 6 commits May 22, 2024 09:58

Fix review comments

097f115

Fix review comments

ea4836b

Fix review comments

a5ed1d2

fix: Remove unused tracker services from program-rule module

11b18b0

Merge remote-tracking branch 'origin/master' into DHIS2-16019

b0fedf5

Fix review comments

8a3d8dc

teleivo reviewed May 22, 2024

View reviewed changes

teleivo approved these changes May 22, 2024

View reviewed changes

enricocolasante added 2 commits May 22, 2024 12:45

Fix review comments

927d790

Fix review comments

5654ec4

github-advanced-security bot found potential problems May 22, 2024

View reviewed changes

enricocolasante merged commit 971003f into master May 22, 2024
15 checks passed

enricocolasante deleted the DHIS2-16019 branch May 22, 2024 11:54

enricocolasante added a commit that referenced this pull request May 30, 2024

feat: Create StartDate and EndDate types [DHIS2-16019] (#17454)

a84ca41

enricocolasante added a commit that referenced this pull request May 30, 2024

feat: Create StartDate and EndDate types [DHIS2-16019] (#17454)

24a0872

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Create StartDate and EndDate types [DHIS2-16019] #17454

feat: Create StartDate and EndDate types [DHIS2-16019] #17454

enricocolasante commented May 16, 2024 •

edited

Loading

teleivo left a comment

teleivo May 17, 2024

enricocolasante commented May 20, 2024

teleivo May 21, 2024

jbee left a comment

jbee May 21, 2024

jbee May 21, 2024

teleivo May 22, 2024

teleivo May 22, 2024

sonarcloud bot commented May 22, 2024

feat: Create StartDate and EndDate types [DHIS2-16019] #17454

feat: Create StartDate and EndDate types [DHIS2-16019] #17454

Conversation

enricocolasante commented May 16, 2024 • edited Loading

teleivo left a comment

Choose a reason for hiding this comment

teleivo May 17, 2024

Choose a reason for hiding this comment

enricocolasante commented May 20, 2024

teleivo May 21, 2024

Choose a reason for hiding this comment

jbee left a comment

Choose a reason for hiding this comment

jbee May 21, 2024

Choose a reason for hiding this comment

jbee May 21, 2024

Choose a reason for hiding this comment

teleivo May 22, 2024

Choose a reason for hiding this comment

teleivo May 22, 2024

Choose a reason for hiding this comment

sonarcloud bot commented May 22, 2024

Quality Gate passed

enricocolasante commented May 16, 2024 •

edited

Loading