OBPIH-6300 Add tests for mapping requestor's dashboard statuses#4585
OBPIH-6300 Add tests for mapping requestor's dashboard statuses#4585awalkowiak merged 2 commits intofeature/upgrade-to-grails-3.3.10from
Conversation
alannadolny
requested review from
awalkowiak,
drodzewicz,
ewaterman and
kchelstowski
|
|
||
| // Because this function needs to handle more than one enum, I left the argument's type as def | ||
| static String getTranslatedDisplayStatus(def status) { | ||
| String getTranslatedDisplayStatus(def status) { |
There was a problem hiding this comment.
I didn't get a chance to review this but could you please change the implementation of this method to be ...
return applicationTagLib.message(code: 'enum.' + status?.class?.simpleName + '.' + status)
Also why do we need a second method to get the translated display status when the original getDisplayStatus() method already does the localization?
There was a problem hiding this comment.
Also why do we need a second method to get the translated display status when the original getDisplayStatus() method already does the localization?
I am not sure if I understand what you mean, but I moved this to a separate function because I wanted to make the code more clean 🤔
| void 'StockMovement.getDisplayStatus() should return: #expected for requisition status #status'() { | ||
| when: | ||
| StockMovement stockMovement = new StockMovement() { | ||
| @Override |
There was a problem hiding this comment.
Nice work on these tests!
In general I'm not the hugest fan of overriding methods in a test like this. Usually I'd try to Stub out the tagLib but it doesn't look like that'd be easy to do since it's fetched statically in the StockMovement. You could maybe Stub the StockMovemnt instance itself with something like:
StockMovement stockMovement = Stub(StockMovement) {
getTranslatedDisplayStatus(def status) >> status.name()
}
but that's essentially what you're doing anyways with your override (and I don't even know if the above code would work). I'm okay with your approach here.
Yay tests!
5 participants
No description provided.