Skip to content
This repository was archived by the owner on Jul 7, 2021. It is now read-only.

#279: add QueryParamEncodedPair#357

Merged
proshin-roman merged 3 commits intomasterfrom
#279-query-param-encoding
Nov 23, 2020
Merged

#279: add QueryParamEncodedPair#357
proshin-roman merged 3 commits intomasterfrom
#279-query-param-encoding

Conversation

@ccostin93
Copy link
Collaborator

No description provided.

@ccostin93 ccostin93 self-assigned this Nov 21, 2020
.withQueryStringParameter("search", "just+a+word")
.withQueryStringParameter("accountTypes", "Checking%2CSecurity")
.withQueryStringParameter("bankConnectionIds", "3%2C4")
.withQueryStringParameter("ids", "1,2")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

When using UriUtils.encodeQueryParam, comma , is allowed in the query params.

I also retested queries against finAPI Sandbox and even with un-encoded comma, the request is correctly interpreted

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

93.2% 93.2% Coverage
0.0% 0.0% Duplication

@codecov
Copy link

codecov bot commented Nov 21, 2020

Codecov Report

Merging #357 (86e9eed) into master (ab8d7c9) will decrease coverage by 0.02%.
The diff coverage is 93.23%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #357      +/-   ##
============================================
- Coverage     83.99%   83.97%   -0.03%     
- Complexity     1035     1039       +4     
============================================
  Files           175      176       +1     
  Lines          2555     2564       +9     
  Branches         20       20              
============================================
+ Hits           2146     2153       +7     
- Misses          391      393       +2     
  Partials         18       18              
Impacted Files Coverage Δ Complexity Δ
.../java/org/proshin/finapi/mandator/FpIbanRules.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...va/org/proshin/finapi/mandator/FpKeywordRules.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
src/main/java/org/proshin/finapi/user/FpUsers.java 100.00% <ø> (ø) 11.00 <0.00> (ø)
...ntial/in/QueryTppAuthenticationGroupsCriteria.java 60.00% <40.00%> (ø) 4.00 <1.00> (ø)
.../tppcredential/in/QueryTppCredentialsCriteria.java 81.81% <66.66%> (ø) 4.00 <1.00> (ø)
.../org/proshin/finapi/mandator/in/UsersCriteria.java 94.87% <93.33%> (ø) 18.00 <12.00> (ø)
...oshin/finapi/account/in/DailyBalancesCriteria.java 100.00% <100.00%> (ø) 10.00 <5.00> (ø)
...org/proshin/finapi/account/in/FpQueryCriteria.java 100.00% <100.00%> (ø) 11.00 <5.00> (ø)
...java/org/proshin/finapi/bank/in/BanksCriteria.java 92.85% <100.00%> (ø) 13.00 <8.00> (ø)
.../proshin/finapi/category/in/CashFlowsCriteria.java 100.00% <100.00%> (ø) 22.00 <17.00> (ø)
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7de1779...2e2e07c. Read the comment docs.

</dependency>
<dependency>
<groupId>org.springframework</groupId>
<artifactId>spring-web</artifactId>
Copy link
Owner

Choose a reason for hiding this comment

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

yeah, my idea was the same - just add this dependency. But maybe there is a smaller lib with the same functionality?

proshin-roman
proshin-roman previously approved these changes Nov 21, 2020
proshin-roman
proshin-roman previously approved these changes Nov 21, 2020
# Conflicts:
#	src/main/java/org/proshin/finapi/transaction/in/DeleteTransactionsCriteria.java
@ccostin93 ccostin93 linked an issue Nov 21, 2020 that may be closed by this pull request
@proshin-roman proshin-roman merged commit 07108fe into master Nov 23, 2020
@proshin-roman proshin-roman deleted the #279-query-param-encoding branch November 23, 2020 09:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bank search not working when using two words split by space

2 participants