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

allow search via post #1176

Open
wants to merge 28 commits into
base: master
Choose a base branch
from
Open

Conversation

hilpitome
Copy link
Contributor

@hilpitome hilpitome commented Nov 16, 2022

addresses issue #1173 for master branch

Comment on lines 84 to 93
String strval = jsonObject.optString(filter);
if (strval.equals("")) {
return null;
}
if (!strval.contains(":")) {
return new DateTime[]{new DateTime(strval), new DateTime(strval)};
}
DateTime d1 = new DateTime(strval.substring(0, strval.indexOf(":")));
DateTime d2 = new DateTime(strval.substring(strval.indexOf(":") + 1));
return new DateTime[]{d1, d2};
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we handle cases of an invalid format for strval?

Comment on lines 264 to 279
public static Integer setCoreFilters(JSONObject jsonObject, ClientSearchBean searchBean) throws ParseException {

Integer limit = !jsonObject.optString("limit").equals("") ? Integer.parseInt(jsonObject.optString("limit"))
: jsonObject.optInt("limit");
if (limit == 0) {
limit = 100;
}

DateTime[] lastEdit = RestUtils.getDateRangeFilter(LAST_UPDATE, jsonObject);//TODO client by provider id
if (lastEdit != null) {
searchBean.setLastEditFrom(lastEdit[0]);
searchBean.setLastEditTo(lastEdit[1]);
}

return limit;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Deduplicate common code in the 2 methods named setCoreFilters

src/main/java/org/opensrp/web/rest/SearchResource.java Outdated Show resolved Hide resolved
}

public static DateTime[] getDateRangeFilter(String filter, Object object) throws ParseException {
String strval;
Copy link
Contributor

Choose a reason for hiding this comment

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

Initialize the variable

}
}
catch (IllegalArgumentException e) {
e.printStackTrace();
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's use the logger for this error

throw new RuntimeException("A required field " + p + " was not found in resource class");
}
catch (IllegalAccessException e) {
e.printStackTrace();
Copy link
Contributor

Choose a reason for hiding this comment

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

See above comment

@@ -92,48 +106,50 @@ public static boolean getBooleanFilter(String filter, HttpServletRequest req) {
public static void main(String[] args) {
System.out.println(new DateTime("​1458932400000"));
Copy link
Contributor

Choose a reason for hiding this comment

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

Unrelated to this update, but should we still have this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure. The code works without that main method

DateTime d2 = Utils.getDateTimeFromString(strval.substring(strval.indexOf(":") + 1));
return new DateTime[] { d1, d2 };
} else {
return null;
Copy link
Contributor

Choose a reason for hiding this comment

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

Where the call happens, what are the implications of returning a null date?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a null check for the date fields e.g

DateTime[] lastEdit = RestUtils.getDateRangeFilter(LAST_UPDATE, jsonObject);//TODO client by provider id

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants