Profile datatype fixes and restrict dynamic profile endpoints to finalised versions #293
Conversation
joe-crawford
commented
Mar 15, 2022
•
joe-crawford
commented
- Treat custom datatypes in profiles as strings (resolves Cannot add a profile, with a data type I have added, to a model #213)
- Fix to loading date formats from ApiProperties (resolves Error validating date field in profile #290)
- Use finalised dynamic profile as default version (resolves Error retrieving default version of profile for dynamic profile with non-finalised version #292), restrict dynamic profiles to finalised only in profile providers endpoint, sort profile provider endpoints
- Show only latest versions (by metadata namespace) of unused profiles by default, add latest versions (by metadata namespace) param to unused and used profile endpoints
- Don't validate profile structure as part of profile `/validate` endpoint - Update profile functional tests
- Choose finalised version as default profile provider version - Sort profile provider endpoints
|
Failing branch test looks like a timing issue. PR includes PR #287. |
There was a problem hiding this comment.
wrt 290 and the not set fields... wow how did i miss that one! bug fail on me
ProfileService -> in 99% of situations where we call any method with the param finalisedOnly we're passing "true" therefore really the default to this param should now be true and the 1 location where we seem to asking for "false" (ProfileController nonProfileMetadata) should be where we pass a param of false...this is a much cleaner approach
...ile/grails-app/controllers/uk/ac/ox/softeng/maurodatamapper/profile/ProfileController.groovy
Outdated
Show resolved
Hide resolved
|
Note to self....need #287 merged first unless this branch is rebased off |
- add ProfileService.validateProfileValues method and add currentValuesOnly to Profile and ProfileSection validate params
- Fix comments on PR 293 - Add latestVersionByMetadataNamespace to used and unused profile endpoints - Make profile providers versionable and sort by default
|
PR has been rebased onto #287 and includes change from review of that PR. |
There was a problem hiding this comment.
Im happy with this.
As a word of note (and it doesnt matter for this case, but will for larger domains) using the "only validate these fields" in the Validateable class is dangerous for larger domains. The reason is that it infact validates the entire domain and then removes all field errors which dont match the requested fields.
Therefore in this case its fine as the domain is small and its not doing any complicated validation, but beware in larger domains with more complex valdiation as its will cascade the validation .
