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

Matching arrays in query params #391

Closed
ryedeer opened this issue Jun 17, 2019 · 4 comments
Closed

Matching arrays in query params #391

ryedeer opened this issue Jun 17, 2019 · 4 comments

Comments

@ryedeer
Copy link
Contributor

ryedeer commented Jun 17, 2019

Hello Daniel!

I've recently upgraded FactoryGuy from 3.4 and found that some of my tests are now failing; namely, the few tests that have arrays in query params for a mock. The mocks defined like mockQuery('user').withSomeParams(by_roles: ["basic.vendor.1"]) are now failing with a message like Error: Pretender intercepted GET http://example.com/api/v1/users?by_roles%5B%5D=basic.vendor.1 but encountered an error: Nothing returned by handler... (which is pretty vague but I see there's another issue for it).

I checked the changes in the code and found the cause of the problem: paramsMatch method in MockRequest uses toParams for the defined mock params but doesn't apply it to the actual query params. To deal with the situation, I've made a quick patch that applied toParams to the both sides:

  paramsMatch(request) {
    if (!isEmptyObject(this.someQueryParams)) {
      return isPartOf(toParams(request.queryParams), toParams(this.someQueryParams));
    }
    if (!isEmptyObject(this.queryParams)) {
      return isEquivalent(toParams(request.queryParams), toParams(this.queryParams));
    }
    return true;
  }

It fixes the array problem and doesn't break any other tests, including the tests for the FactoryGuy itself. So I think it would be a good idea to use this update in the library.

Thank you!

@danielspaniel
Copy link
Collaborator

@ryedeer .. good catch, and I think that is good fix. Did I not have test for array params ( I am sloppy if not cause that is important one ). Can you submit PR with this fix and maybe for the sake of our sanity add a test for array params ? if there is not one yet

ryedeer added a commit to Aelloy/ember-data-factory-guy that referenced this issue Jun 18, 2019
@ryedeer
Copy link
Contributor Author

ryedeer commented Jun 18, 2019

Sure :)

@danielspaniel
Copy link
Collaborator

v 3.9.2 i just released with this PR in it

@ryedeer
Copy link
Contributor Author

ryedeer commented Jun 18, 2019

Thank you very much! It helps a lot.

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

No branches or pull requests

2 participants