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

feat: add target field placeholder #2363

Merged
merged 5 commits into from Sep 22, 2019

Conversation

davestewart
Copy link
Collaborator

@davestewart davestewart commented Sep 22, 2019

πŸ”Ž Overview

This PR adds functionality to show any related target field names using {_target_} or for multiple targets {_fooTarget_}, {_barTarget_}, ...

  test('fills target field placeholder', async () => {
    const names = { foo: 'Foo', bar: 'Bar', baz: 'Baz' };
    const values = { foo: 10, bar: 20 };
    const rules = 'confirmed:foo';
    const options = {
      name: names.bar,
      values,
      names
    };
    const result = await validate(values.bar, rules, options);
    expect(result.errors[0]).toEqual('Bar must match Foo');
  });

See tests in:

βœ” Issues affected

closes #2290

@codecov
Copy link

codecov bot commented Sep 22, 2019

Codecov Report

Merging #2363 into master will increase coverage by 0.04%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2363      +/-   ##
==========================================
+ Coverage   95.19%   95.24%   +0.04%     
==========================================
  Files          44       44              
  Lines        1041     1051      +10     
  Branches      154      155       +1     
==========================================
+ Hits          991     1001      +10     
  Misses         50       50
Impacted Files Coverage Ξ”
src/validate.ts 96.49% <100%> (+0.33%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Ξ” = absolute <relative> (impact), ΓΈ = not affected, ? = missing data
Powered by Codecov. Last update f535616...8c21df3. Read the comment docs.

@davestewart
Copy link
Collaborator Author

davestewart commented Sep 22, 2019

I can update this PR to support multiple isTarget parameters if needed:

Code:

function _getTargetName(
  field: FieldContext,
  ruleSchema: ValidationRuleSchema,
  ruleName: string
): Record<string, string> {
  if (ruleSchema.params) {
    const hasMultiple = ruleSchema.params.filter(param => (param as RuleParamConfig).isTarget).length > 1;
    const names: Record<string, string> = {};
    for (let index = 0; index < ruleSchema.params.length; index++) {
      const param: RuleParamConfig = ruleSchema.params[index] as RuleParamConfig;
      if (param.isTarget) {
        const key = field.rules[ruleName][index];
        const name = field.names[key] || key;
        if (hasMultiple) {
          names[`_${param.name}Target_`] = name;
        } else {
          names._target_ = name;
        }
      }
    }
    return names;
  }
  return {};
}

Test:

  test('works for multiple targets', async () => {
    extend('sum_of', {
      message: '{_field_} must be the sum of {_aTarget_} and {_bTarget_}',
      params: [
        { name: 'a', isTarget: true },
        { name: 'b', isTarget: true }
      ],
      validate: (value, { a, b }) => value === parseInt(a, 10) + parseInt(b, 10)
    });

    const values = { foo: 10, bar: 10, baz: 10 };
    const names = { foo: 'Foo', bar: 'Bar', baz: 'Baz' };
    const rules = 'sum_of:bar,baz';
    const options = {
      name: names.foo,
      values,
      names
    };

    const result = await validate(values.foo, rules, options);
    expect(result.errors[0]).toEqual('Foo must be the sum of Bar and Baz');
  });

Let me know if you want this pushed as well.

@davestewart davestewart changed the title Feature/add target placeholder feat: add target field placeholder Sep 22, 2019
@logaretm
Copy link
Owner

That would be great as well, will merge when you push it. Thank you!

@davestewart
Copy link
Collaborator Author

OK, pushed!

@logaretm logaretm merged commit b905b85 into logaretm:master Sep 22, 2019
@davestewart davestewart deleted the feature/add-target-placeholder branch September 22, 2019 20:23
@davestewart
Copy link
Collaborator Author

Nice one!

Any idea when these changes will be available on NPM?

@logaretm
Copy link
Owner

Just tagged 3.0.7 πŸ˜€

@davestewart
Copy link
Collaborator Author

Very much appreciated - big push for an initial release at work this week, and validation is one of the last items to tick off!

Thank you :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide target parameter field names as message placeholders
2 participants