cannercms icon indicating copy to clipboard operation
cannercms copied to clipboard

Improve validation with asynchronous pipeline

Open Mosoc opened this issue 6 years ago • 4 comments

As reported in #173 #174

There are some todo:

  • [x] Add schema property to validation object
  • [x] Only create Ajv instance when schema is verified as non-empty object
  • [x] Completion of type definition
  • [x] Implement asynchronous structure validation
  • [x] Make customize support both (a)sync function
  • [x] Check valid or invalid situation
  • [x] Handle errors by promise chaining
  • [x] Update and run test cases
  • [x] Documentation

Mosoc avatar Apr 30 '19 15:04 Mosoc

@abz53378 My current solution is: Add async syntax to componentDidMount with await validate function resolution.

    async componentDidMount() {
      const {refId, validation = {}, onDeploy, required = false} = this.props;
      if (isEmpty(validation) && !required) {
        // no validation
        return;
      }
      const key = refId.getPathArr()[0];
      this.callbackId = onDeploy(key, await this.validate);
    }

Then run test with waiting this lifecycle, is it a good way?

   it('should onDeploy be called', async () => {
    const wrapper =  mount(<WrapperComponent
      {...props}
      required
    />);
    const instance = wrapper.instance();
    await instance.componentDidMount();
    expect(onDeploy).toBeCalledWith('posts', wrapper.instance().validate);
  });

Mosoc avatar May 15 '19 06:05 Mosoc

@Mosoc , I don't think it's a good way to make the componentDidMount async, since the onDeploy function actually is just used to register the callback, not execute the callback. It's same as the registerCallback

abz53378 avatar May 15 '19 06:05 abz53378

@abz53378 Finally, I update the test files as your suggestions, and add new cases about async functions except unexpected error one. But I tested it manually by following snippet and it worked.

  it('should use customized validator with unexpected error', async () => {
    const result = {
      data: {
        0: { url: ''}
      }
    };
    const wrapper =  mount(<WrapperComponent
      {...props}
      validation={
        {
          validator: () => {
            const value = 1;
            value = 2;
          }
        }
      }
    />);
    await wrapper.instance().validate(result)
    expect(wrapper.state()).toMatchObject({
      error: true,
      errorInfo: [{
        message: 'Error: "value" is read-only'
      }]
    })
  });

Mosoc avatar May 16 '19 00:05 Mosoc

I rechecked, and found that only one place needs to be modified.

In docs folder, there is a galleryValidation.

export const galleryValidation = {
  validator: (content, reject) => {
    if (content.length === 0) {
      return reject("should at least have one photo");
    }
  }
};

Maybe it could be following snippet in new version:

export const galleryValidation = {
  validator: (content) => {
    if (content.length === 0) {
      return "should at least have one photo";
    }
  }
};

Mosoc avatar May 18 '19 06:05 Mosoc