math-dojo / user-account-service

Microservice for managing the users of the math-dojo platform
1 stars 0 forks source link

Ft9 Part1: Add GET orgById, POST to createOrg & Integration Test Suite #14

Closed noce2cicd closed 4 years ago

noce2cicd commented 4 years ago

Purpose

Does this introduce a breaking change?

[ ] Yes
[X] No

Pull Request Type

What kind of change does this Pull Request introduce?

[ ] Bugfix
[X] Feature
[ ] Code style update (formatting, local variables)
[X] Refactoring (no functional changes, no api changes)
[ ] Documentation content changes
[ ] Other... Please describe:

How to Test

git clone [repo-address]
cd [repo-name]
git checkout [branch-name]

What to Check

Verify that the following are valid

............... Terminating the functions process running with id 22727

4 scenarios (4 passed) 11 steps (11 passed)



## Other Information
<!-- Add any other helpful information that may be needed here. -->
noce2 commented 4 years ago

I have a question about your use of singleton classes for some validator/verifier classes. Why would it not be better to make those instance/static variables in the class they are needed and effectively trust spring to enforce their singleton nature? Apart from that I left a few minor questions. I'll approve once clarified, good work overall :-)

Ah what do you mean? Could you make a code suggestion as a comment on one of the ones you had in mind? Did you mean like this? https://github.com/math-dojo/user-account-service/pull/14/files#r419880929

If so, I had a lot of problems trying to do that. The function bean with the injected class ended up not getting registered for some reason. I did quite a bit of digging at the time but this (i.e. directly referencing the needed object in the function to be returned as a Bean seemed to work). ¯_(ツ)_/¯