Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

editor: add custom validators support #354

Merged
merged 1 commit into from
Mar 3, 2021

Conversation

jma
Copy link
Contributor

@jma jma commented Feb 23, 2021

  • Adds custom validators support for repeatable fields.
  • Fixes missing duplicate and trash support for form field in non
    long mode.

Co-Authored-by: Johnny Mariéthoz Johnny.Mariethoz@rero.ch

Why are you opening this PR?

  • Which task/US does it implement?
  • Which issue does it fix?

How to test?

  • What command should I have to run to test your PR?
  • What should I test through the UI?

Code review check list

  • Commit message template compliance.
  • Commit message without typos.
  • File names.
  • Functions names.
  • Functions docstrings.
  • Unnecessary commited files?
  • Extracted translations?

@jma jma force-pushed the maj-custom-validators branch from e6e1fc6 to cc3fb6a Compare February 23, 2021 09:23
@jma jma marked this pull request as ready for review February 23, 2021 09:24
@iGormilhit iGormilhit added the f: editor Concerns editor based on JSON schema AND custom editor label Feb 23, 2021
@jma jma requested review from sebdeleze and zannkukai February 24, 2021 13:39
Comment on lines 682 to 683
for (const customValidator of this._customValidators) {
if (formOptions.validation && formOptions.validation.validators) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The if should be placed before the for isn't it ?

* @returns number - the index position in the parent array, null if the parent
* is not an array.
*/
getIndex() {

Choose a reason for hiding this comment

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

If think this method is useless and you can do directly Number(this.field.key) instead of using it, because the check if field is array is already done

@@ -48,7 +51,7 @@ export class NgCoreFormlyExtension {
* Constructor
* @param _editorService - editor service
*/
constructor(private _editorService: EditorService) { }
constructor(private _editorService: EditorService, private _recordService: RecordService) { }

Choose a reason for hiding this comment

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

Update doc block

* Adds custom validators support for repeatable fields.
* Fixes missing duplicate and trash support for form field in non
  long mode.

Co-Authored-by: Johnny Mariéthoz <Johnny.Mariethoz@rero.ch>
@jma jma force-pushed the maj-custom-validators branch from cc3fb6a to 4f8e870 Compare February 25, 2021 13:55
@jma jma requested review from zannkukai and sebdeleze February 25, 2021 13:56
@@ -47,8 +50,11 @@ export class NgCoreFormlyExtension {
/**
* Constructor
* @param _editorService - editor service
* @params _recordService - ng core record service
Copy link
Contributor

Choose a reason for hiding this comment

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

typo : @params or @param

Comment on lines +197 to +202
const remoteRecordType =
customValidators.valueAlreadyExists.remoteRecordType;
const limitToValues =
customValidators.valueAlreadyExists.limitToValues;
const filter =
customValidators.valueAlreadyExists.filter;
Copy link
Contributor

Choose a reason for hiding this comment

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

cosmetic : spit the lines here seems confused (for me and myself)

Copy link

@iGormilhit iGormilhit left a comment

Choose a reason for hiding this comment

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

Commit message approved.

@iGormilhit iGormilhit added this to the v1.2.0 milestone Mar 1, 2021
@sebdeleze sebdeleze merged commit 8f11056 into rero:dev Mar 3, 2021
@jma jma deleted the maj-custom-validators branch December 13, 2021 10:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
f: editor Concerns editor based on JSON schema AND custom editor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants