Feature/refactor example default #3816
Conversation
Fill default values for properties on application of patch/modules Fix jhipster#3480
Codecov ReportBase: 100.00% // Head: 100.00% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## main #3816 +/- ##
============================================
Coverage 100.00% 100.00%
- Complexity 2094 2158 +64
============================================
Files 545 565 +20
Lines 8935 9264 +329
Branches 175 179 +4
============================================
+ Hits 8935 9264 +329
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
| @@ -117,8 +113,8 @@ public Optional<JHipsterPropertyDescription> description() { | |||
| return description; | |||
| } | |||
|
|
|||
| public Optional<JHipsterPropertyExample> example() { | |||
There was a problem hiding this comment.
Need to keep it deprecated at least for 1 version. See JHipsterModuleReplacements.JHipsterModuleReplacementsBuilder.in for deprecation example.
Doing that since it can be used in custom JHLite instances
There was a problem hiding this comment.
@DamnClin Sorry I am not really getting this one, what do you mean by "custom JHLite instances" here and how would they be affected by this change?
Is it for JHLite being used as a dependency in other project? Not sure how it affects generated apps.
There was a problem hiding this comment.
Yep. You have a module called custom-jhlite that generate instances allowing organization to build there own module. In this we have dependencies to JHLite so we need to give them some time to adapt to new apis
There was a problem hiding this comment.
Thanks, i have kept the example in here and builder as deprecated.
| case 'BOOLEAN': | ||
| updateProperty(createProperty(property.key, Boolean(property.defaultValue))); | ||
| break; | ||
| default: |
There was a problem hiding this comment.
You can avoid default and have code not compiling without all values by putting that in a dedicated method. see modeClass in Landscape.component.ts for an example
There was a problem hiding this comment.
Sure, I would rather do a common util method in PropertyValue.ts for use in both Landscape and PathModule
| @@ -205,6 +205,21 @@ export default defineComponent({ | |||
| const applyModule = (module: string): void => { | |||
| operationInProgress.value = true; | |||
|
|
|||
| selectedModuleProperties().forEach(property => { | |||
There was a problem hiding this comment.
This looks a lot like a duplication :)
There was a problem hiding this comment.
:), would reuse the common method from PropertyValue.ts for casting logic.
Reduce redundancy and move to common castValue method Fix jhipster#3480
Added deprecated example method in property definition and builder Fix jhipster#3480
| public Optional<JHipsterPropertyExample> example() { | ||
| return example; | ||
| if (defaultValue.isPresent()) { |
There was a problem hiding this comment.
defaultValue.map(value -> JHipsterPropertyExample.of(value.get()));
There was a problem hiding this comment.
Thanks, I see it has to be flatMap , else it would return Optional<Optional<JhipsterPropertyExample>>, so:-
defaultValue.flatMap(value -> JHipsterPropertyExample.of(value.get()));
|
Had to add a test case for deprecated method, my assumption was its ignored from Codecov. |
|
Good job here. cc @pascalgrimaud I think this deserve a Bounty :) |
Fill default values for properties on application of patch/modules
Fix #3480