Skip to content

Fix: Invoice Search parameters, #31

Merged
Aman-Aalam merged 5 commits intomasterfrom
dev
Aug 24, 2023
Merged

Fix: Invoice Search parameters, #31
Aman-Aalam merged 5 commits intomasterfrom
dev

Conversation

@Aman-Aalam
Copy link
Contributor

@Aman-Aalam Aman-Aalam commented Aug 21, 2023

Fixes

  • Fix how parameters are sent while searching for Invoice
  • Optimized imports
  • Improved tests around Invoice pagination and search

Checklist

  • I acknowledge that all my contributions will be made under the project's license
  • I have made a material change to the repo (functionality, testing, spelling, grammar)
  • I have titled the PR appropriately
  • I have updated my branch with the main branch
  • I have added tests that prove my fix is effective or that my feature works
  • I have added the necessary documentation about the functionality in the appropriate .md file
  • I have added inline documentation to the code I modified

If you have questions, create a GitHub Issue in this repository.

mortondaniel and others added 5 commits August 18, 2023 13:47
fix: Update searchInvoices to use array for external ids + use lower camel case for search param
…he Invoice.SearchBy Enum, we take its key into account
@Aman-Aalam Aman-Aalam requested a review from LukasBil August 21, 2023 22:12
Comment on lines 148 to 153
}
body = "{\""+searchBy.name()+"\":"
body = "{\""+searchBy.getKey()+"\":"
+new ObjectMapper()
.setSerializationInclusion(Include.NON_EMPTY)
.writeValueAsString(paramsList)
+"}";

Choose a reason for hiding this comment

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

Instead of building it by hand, maybe you would like to do it somehow like this:

ObjectMapper objectMapper = new ObjectMapper().setSerializationInclusion(JsonInclude.Include.NON_EMPTY);
body = objectMapper.writeValueAsString(Map.of(searchBy.getKey(), paramsList));

@Aman-Aalam Aman-Aalam merged commit bb860a7 into master Aug 24, 2023
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.

3 participants