openmrs-core
openmrs-core copied to clipboard
Trunk 6124 Eliminated unnecessary methods (toDatetime(), getDatetime(), and getDate() should all do the same thing)
Description of what I changed
Eliminated unnecessary methods (toDatetime(), getDatetime(), and getDate() should all do the same thing)
Issue I worked on
https://openmrs.atlassian.net/browse/TRUNK-6124
Checklist: I completed these to help reviewers :)
-
[X] My IDE is configured to follow the code style of this project.
-
[X] I ran
mvn clean package
right before creating this pull request and added all formatting changes to my commit. -
[X] All new and existing tests passed.
-
[X] My pull request is based on the latest changes of the master branch.
@subhamkumarr change the title to reflect what's being done in the ticket
@tendomart added
@tendomart should i also add test to the code?
import org.junit.Test; import static org.junit.Assert.*;
public class ResultTest {
@Test
public void testToDatetimeWithNonNullValueDatetime() {
// Create a Result with a non-null valueDatetime
Result result = new Result(new Date(), Result.Datatype.DATETIME, null, null, new Date(), null, null, null);
// Call toDatetime and assert the expected result
assertEquals(result.getValueDatetime(), result.toDatetime());
}
@Test
public void testToDatetimeWithTextValue() {
// Create a Result with datatype TEXT and a non-null valueText that can be parsed into a date
String dateString = "2022-01-01";
Result result = new Result(new Date(), Result.Datatype.TEXT, null, null, null, null, dateString, null);
// Call toDatetime and assert the expected result
assertEquals(DateUtils.parseDate(dateString), result.toDatetime());
}
@Test
public void testToDatetimeWithTextValueParsingError() {
// Create a Result with datatype TEXT and a valueText that cannot be parsed into a date
Result result = new Result(new Date(), Result.Datatype.TEXT, null, null, null, null, "invalidDate", null);
// Call toDatetime and assert that it returns null
assertNull(result.toDatetime());
}
@Test
public void testToDatetimeWithMultipleResults() {
// Create a Result with multiple sub-results, each having a different valueDatetime
Result subResult1 = new Result(new Date(), Result.Datatype.DATETIME, null, null, new Date(), null, null, null);
Result subResult2 = new Result(new Date(), Result.Datatype.DATETIME, null, null, DateUtils.addDays(new Date(), 1), null, null, null);
Result result = new Result(Arrays.asList(subResult1, subResult2));
// Call toDatetime and assert that it returns the earliest valueDatetime among sub-results
assertEquals(subResult1.toDatetime(), result.toDatetime());
}
}
Yes please, just to make sure nothing is breaking.
@tendomart Sir everything is good from my end. Please merge it.
yes @subhamkumarr line 25
import static org.junit.Assert.*;
avoid imports like which end with .*, always import a specific class that you're using, avoid importing an entire package
look at this https://wiki.openmrs.org/display/docs/Java+Conventions
in short , use onlly absolute paths
@tendomart done the changes. Could you please review and merge.
@subhamkumarr have you responded to all the changes which I requested above ?
@tendomart I made all the changes you asked me to do. Please review.
@tendomart is this good for merge?
@dkayiwa could you please review my PR.
You have some failing tests..
do you need help in that ?
Error: Errors:
Error: AllergyValidatorTest.validate_shouldFailIfAllergenIsNull:82 » NullPointer Allergen must not be null
Error: AllergyValidatorTest.validate_shouldFailIfPatientIsNull:73 » NullPointer Allergen must not be null
Error: AllergyValidatorTest.validate_shouldRejectNumericReactionValue:191 » NullPointer Allergen must not be null
@tendomart yes i need help
@tendomart what changes i need to make?
tl;dr our action detected no activity on this PR and will close it in 30 days if the stale label is not removed.
OpenMRS welcomes your contribution! It means a lot to us that you want to contribute to equity in healthcare!
This PR has not seen any activity in the last 5 months. That is why we wanted to check whether you are still working on it or need assistance from our side. Please note that this is an automated message and we might very well be the reason why there has not been any activity lately. We certainly do not want to discourage you from contributing. We do need to be honest in that OpenMRS has limited resources for reviewing PRs.
If you do not have time to continue the work or have moved on you don’t need to do anything. We will automatically close the PR in 30 days. We hope to see you back soon :) If you would like to continue working on it or require help from us please remove the stale label and respond by commenting on the issue.