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

[RFR] Ability to use custom method for get requests #555

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

[RFR] Ability to use custom method for get requests #555

wants to merge 10 commits into from

Conversation

lukemarsh
Copy link

I have just submitted a pull request for your admin-config module for adding a custom method to get requests. I have changed getOne and getList functions to allow for the custom methods

@fzaninotto
Copy link
Member

Could you document the change in the docs? (cf 54f50e8)

@fzaninotto
Copy link
Member

Refs marmelab/admin-config#24

@jpetitcolas
Copy link
Contributor

Good to me. Waiting for marmelab/admin-config#24 PR to be merged.

@jpetitcolas
Copy link
Contributor

Can you just rebase on master to ensure all tests are green? We fixed them yesterday. :)

@jpetitcolas jpetitcolas changed the title Ability to use custom method for get requests [RFR] Ability to use custom method for get requests Jul 24, 2015
@fzaninotto
Copy link
Member

Your PR absolutely doesn't work. Did you even test it? The restangular code doesn't make sense.

Please, test your code before submitting it (e.g. by setting the retrieveMethod to 'GET' to make sure it doesn't break anything), I just spent half an hour trying to make this work.

@fzaninotto fzaninotto mentioned this pull request Jul 28, 2015
@lukemarsh
Copy link
Author

Apologies for wasting your time on this one! I have since fixed said issues but the tests were failing due to some dependency issues, which I have fixed with this PR: #585

@fzaninotto
Copy link
Member

Did you test with a simple posts.retrieveMethod('GET') in the example to check that it works?

Luke Marsh added 2 commits August 5, 2015 10:15
…-method-for-get-one-and-get-list

Conflicts:
	build/ng-admin-only.min.js
	build/ng-admin.min.js
@lukemarsh
Copy link
Author

Yes I did and it works :) but the method needs to be lowercase!

@CKUDG
Copy link

CKUDG commented Sep 7, 2015

Is this now fully implemented and works now?

I would also really need that feature to force POST method on the getList()...

Thanks much much and best regards;
C

@mosasiru
Copy link

mosasiru commented Jan 31, 2017

Did you test with a simple posts.retrieveMethod('GET') in the example to check that it works?

It seems that posts.retrieveMethod('get') changesgetList operation to 'get', so it does not work.
This test is edge case, because many uses case is that both getList and get operation are wanted to be POST method, not GET method.
If you want it, I will write pull-request to admin-config that devides retrieveMethod into getMethod and getListMethod, but It would be too much.
or I can just fix conflict of this PR. Which do you prefer?

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.

5 participants