seriema / angular-apimock

Automatically route your API calls to static JSON files, for hiccup free front–end development.
johansson.jp/angular-apimock
MIT License
65 stars 8 forks source link

Handle queries (i.e. "?name=Pikachu") #23

Closed seriema closed 9 years ago

seriema commented 9 years ago

Currently query parameters in the API URL are just stripped out. There should be an optional setting for including them in the redirect.

Use case Better testing of search-like features where one could test search terms that will give different search results, such as no results, a few results, a lot of results, spelling suggestions, etc.

Usage Config set in the config() by setting stripQueries to false (default is true). Sample:

angular.module('testApp', ['apiMock'])
.config(function (apiMockProvider) {
  apiMockProvider.config({
    stripQueries: false
  });
});

Descoped (do this later as a separate feature): Local and global flag set on individual requests, which would be perfect for isolated testing of a autocompletion type search. To not pollute the $http config we should scope it within apiMock, but that requires another feature (to allow config objects instead of just true/false). Sample:

app.service('exampleService', function ($http) {
  // ...
  $http({
    method: 'GET',
    url: '...',
    apiMock: {
      stripQueries: true
    }
  }) // ...
  // ...
});

Effect

trevcor commented 9 years ago

Still trying to steal some time away to work on this. Hopefully be done this week.

seriema commented 9 years ago

No problem! I'd love to hear your comments on the questions above.

trevcor commented 9 years ago

Had some time to look around more of the code and think about your questions.

  1. I agree we should lowercase the query values by default but give a config to override that behavior. I think the query param name should stay as it is. Should we call the config forceLowerCaseQueries and default to true?
  2. Yep I like this pattern way better than the underscore.
  3. Agreed. Default to true makes more sense.
  4. Yeah this makes sense.
  5. I'm still looking at that commit. When I view it in github it looks like the entire file was changed so its tough to see what exactly the change was.
  6. I also agree with this. Let's get the global stuff working and then we can add the targeted api feature later.

So for the global feature we will add 2 config options, forceLowerCaseQueries, stripQueries.

Anything else?

seriema commented 9 years ago
  1. If the request comes up we can add the flag. I can't find a good use case for mixed casings and it would just add more code/tests unnecessarily. For now let's just have lowercase as default.
  2. Yeah it's annoying (hence why I request clean diffs in the contribution guidelines now), but ctrl+f for "appendParamsToPath" and you should see it.

Thanks!

seriema commented 9 years ago

If anyone wants to discuss this feature, I've opened a Gitter chatroom.