Skip to content
This repository was archived by the owner on Nov 19, 2024. It is now read-only.

Issue #206: PaymentController can only respond with 404 or with NullPointerException when creating Payment#210

Merged
mslowiak merged 2 commits intojmprathab:masterfrom
edwardUL99:bugfix/issue-206-PaymentController-Null-Exception
Feb 20, 2021
Merged

Issue #206: PaymentController can only respond with 404 or with NullPointerException when creating Payment#210
mslowiak merged 2 commits intojmprathab:masterfrom
edwardUL99:bugfix/issue-206-PaymentController-Null-Exception

Conversation

@edwardUL99
Copy link
Collaborator

I didn't realise I had to name the commit with the specified pattern, sorry!

🚀 Description

This PR closes #206.

It has the following changes:

  • An AfterMapping method has been added to SchedulePaymentApiMapper so that after mapping a payment request to an enriched one, it fills in the null admin and member which were causing the NullPointerExceptions.
  • A ManyToOne relationship also had to be added to admin and member in Payment.java, as without them, there were issues with fields being too long
  • Payment#description was defined as unique but this was causing issues after fixing the other issue. It makes sense that a description should not be unique

📄 Motivation and Context

This big fix resolves the NullPointerExceptions blocking pull request #205. It closes #206

🧪 How Has This Been Tested?

Since pull request #205 has not been merged yet, I copied all the required code over to my branch to re-run the failing test. I removed the Disabled annotation and ran the test and it passed.

Also used Postman to test the scenario of creating a payment. I tried the scenario of creating a payment with an admin ID that is not in the community of the member ID and got the 404 response because it couldn't find that admin in the member's community.

📷 Screenshots (if appropriate)

📦 Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

✅ Checklist

  • My code follows the code style of this project(Do your best to follow code styles. If none apply just skip this).
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

…rDto and member details to HouseMemberDto to resolve the null exceptions. Made Payment#description non-unique
@edwardUL99 edwardUL99 requested a review from mslowiak December 8, 2020 21:05
@codecov
Copy link

codecov bot commented Dec 8, 2020

Codecov Report

Merging #210 (2afceac) into master (91ff17c) will decrease coverage by 0.88%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #210      +/-   ##
============================================
- Coverage     69.01%   68.12%   -0.89%     
- Complexity      198      200       +2     
============================================
  Files            32       34       +2     
  Lines           823      866      +43     
  Branches         37       41       +4     
============================================
+ Hits            568      590      +22     
- Misses          244      268      +24     
+ Partials         11        8       -3     
Impacted Files Coverage Δ Complexity Δ
...in/java/com/myhome/controllers/UserController.java 80.43% <0.00%> (-16.94%) 12.00% <0.00%> (ø%)
...yhome/services/springdatajpa/UserSDJpaService.java 77.38% <0.00%> (-15.08%) 27.00% <0.00%> (+8.00%) ⬇️
...src/main/java/com/myhome/security/WebSecurity.java 0.00% <0.00%> (ø) 0.00% <0.00%> (ø%)
...java/com/myhome/controllers/AmenityController.java 100.00% <0.00%> (ø) 8.00% <0.00%> (ø%)
...java/com/myhome/controllers/PaymentController.java 100.00% <0.00%> (ø) 12.00% <0.00%> (+2.00%)
...ome/controllers/HouseMemberDocumentController.java 100.00% <0.00%> (ø) 12.00% <0.00%> (-5.00%)
...me/services/springdatajpa/PaymentSDJpaService.java 0.00% <0.00%> (ø) 0.00% <0.00%> (ø%)
...java/com/myhome/security/AppUserDetailFetcher.java
...n/java/com/myhome/configuration/OpenApiConfig.java
...om/myhome/security/MyHomeAuthenticationFilter.java
... and 7 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 610a129...2afceac. Read the comment docs.

@mslowiak
Copy link
Collaborator

mslowiak commented Feb 17, 2021

A ManyToOne relationship also had to be added to admin and member in Payment.java, as without them, there were issues with fields being too long

@edwardUL99
Can you elaborate on that? What means 'fields being too long'?
BTW. Sorry for the late response on that

@edwardUL99
Copy link
Collaborator Author

A ManyToOne relationship also had to be added to admin and member in Payment.java, as without them, there were issues with fields being too long

@edwardUL99
Can you elaborate on that? What means 'fields being too long'?
BTW. Sorry for the late response on that

I don't really remember now, in hindsight should have documted the stack trace. I believe it was an exception with IDs being greater than 255 characters. Some reason adding ManyToOne fixed that, I'm not too sure.

Copy link
Collaborator

@mslowiak mslowiak left a comment

Choose a reason for hiding this comment

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

There are small changes but if you correct them we can merge that @edwardUL99

@mslowiak mslowiak added the awaiting-review-changes Awaiting for PR owner to make changes label Feb 20, 2021
@mslowiak mslowiak merged commit 48235a7 into jmprathab:master Feb 20, 2021
@mslowiak
Copy link
Collaborator

Thanks @edwardUL99

@edwardUL99
Copy link
Collaborator Author

No problem @mslowiak

@mslowiak
Copy link
Collaborator

Feel free to take something new, there are plenty of new issues @edwardUL99 ! :)

@mslowiak mslowiak removed the awaiting-review-changes Awaiting for PR owner to make changes label Feb 20, 2021
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.

PaymentController can only respond with 404 or with NullPointerException when creating Payment

2 participants