Mock URL matching not working with query params

[patocallaghan]

The following mockUpdate url matching fails if the url to match contains any query params.

basicRequestMatches(settings) {
    /**
     If no id is specified, match any url with an id,
     with or without a trailing slash after the id.
     Ex: /profiles/:id and /profiles/:id/
     */
    let url = this.getUrl();
    if (!this.id) {
      url = new RegExp(url + '\/*\\d+\/*');
    }

    // settings.url.match(url) FAILS BECAUSE ? IS A SPECIAL REGEX CHARACTER
    return settings.url.match(url) && settings.type === this.getType();
  }

In our case we add a query param to all our requests in our adapter so our PUT url looks something like /ember/campaigns/1?app_id=abc123 with the additional app_id. If you do some pre-processing to escape the string for regex special characters the match works again.

I can open a PR with a failing test or possible fix later but wanted to open this anyway.

[patocallaghan]

I believe this will also affect mockDelete as that does something similar

[danielspaniel]

Yes it will affect both. I was thinking there was a better way to check the url than what is going on now .. so if you get a better way .. bring it on.
In other words I think there is a way to do this without the regex. I had almost had a way to do it but I did not quite finish the idea.

[patocallaghan]

@danielspaniel I don't really have enough context on all the different codepaths through here to know how to do it without a regex.

Would there be a way to do something similar to buildURL for the URL we pass when registering the mock? That way we should be able to do a simple comparison. Saying that, that probably wouldn't work for mockDelete('campaign') though as we don't have the id in advance.

[danielspaniel]

For now, then go with your original idea .. if it works .. fine by me .. we can get fancy later ( whenever I remember the good idea I had )

[patocallaghan]

I've been looking at adding a test by adding a custom adapter && buildUrl for the employee model. Can't seem to get the adapter to be picked up in my test though. I've tried to add it to tests/unit/shared-factory-guy-test-helper-tests.js but there seems to be some adapter magic going on there. Is that the correct place it should go?

[patocallaghan]

For context this is the test I'm trying to get to fail master...patocallaghan:patoc/mock_delete_update_param

[danielspaniel]

setting up the adapter like that might not work because in the shared test concept .. before the tests are run, in the helpers/utility-methods.js you will see a method theUsualSetup
where all models are filtered and assigned a serializer and adapter ( depending on the test environment , json, jsonapi, rest, ams ..etc ) ..

so your buildUrl method is being ignored for that reason.

The workaround is not so easy ... if you look at the way I am adding things to serializers ( serializerOptions ) .. you can see maybe one way around this solution ( which I would probably use to make adapterOptions .. add a buildUrl method for employee and slap that sucker into the adapter when it's assigned like I do with serializers .

If you can't figure it out ( and I don't expect you too .. since it is an extra ordinary amount of hackery ) .. I can merge this and finish that part ( don't mind at all actually ..since I might as well continue the hackerooskyness in my own crazy style ? ) so whatever you like.

[patocallaghan]

yeah if you want to continue the hackery then go ahead :D I can do up the fix and test it locally myself

[danielspaniel]

got it .. should i merge this then?

[patocallaghan]

sure 👍

[danielspaniel]

fixed => https://github.com/danielspaniel/ember-data-factory-guy/releases/tag/v2.7.6