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

Support for complex object in ETag configuration (useful for MongoDB $oid) #1359

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

Conversation

dokkis
Copy link

@dokkis dokkis commented Apr 8, 2016

Similar support implemented for id tags, to support the MongoDB $oid also into the etag and not only in the id field by configuring the RestangularProvider as following:

RestangularProvider.setRestangularFields({
etag: '_etag.$oid'
});

Please integrate this pull request or resolve the problem in any other kind of way! :)

Thank you.

…lemented for id tags, to support the MongoDB $oid also into the etag by configuring the RestangularProvider as following:

RestangularProvider.setRestangularFields({
   etag: '_etag.$oid'
});
@daviesgeek
Copy link
Collaborator

Would you mind submitting some tests as well? I'll take a look at this, but it's always helpful to have tests with your PR.

@dokkis
Copy link
Author

dokkis commented Apr 9, 2016

Hi, thank you for your fast answer. I added 4 tests:

testing ETag header
✔ Should GET an object with returning header ETag and set the ETag header value inside a simple etag attribute
✔ Should GET an object with returning header ETag and set the ETag header value inside a complex etag attribute
testing If-Match header to support ETag
✔ Should put the If-Match header with value retrieved by a simple etag attribute
✔ Should put the If-Match header with value retrieved by a complex etag attribute

testing ETag header tests that the ETag header is correctly retrieved by the response headers andits value is injected into the object (in the _etag attribute in the first test and _etag.$oid attribute in the second one). The first one was supported before, infact it's a regression test, the second test shows that with my improvements now Restangular can fill also complex object). The support is really similar to the 'id' field, I just made the things the same as for the id field.

testing If-Match header to support ETag tests that the If-Match header is populated correctly if it's declared inside the object. In the first test the etag is inside _etag attribute. For the second one is inside the _etag.$oid.

Hoping that this pull request will be integrated as soon as possible.

Thank you.

@daviesgeek
Copy link
Collaborator

Awesome thanks. I'll see what I can do about merging this.

@daviesgeek
Copy link
Collaborator

One more thing, sorry. Would you mind updating the docs to include the changes you made in this PR?

@dokkis
Copy link
Author

dokkis commented Apr 9, 2016

Made it, I added into the FAQ after the "I use Mongo and the ID of the elements is _id not id as the default. Therefore requests are sent to undefined routes" because they are really close problems ;)

@daviesgeek
Copy link
Collaborator

Great thanks. Can you squash those three readme commits into one?

I'm waiting for the other contributors to review this before merging.

@dokkis
Copy link
Author

dokkis commented Apr 9, 2016

Done the squash of the three last commits! Hope for good news about merging soon ;) In a project I had to invalidate etag support of restangular and introduce the etag support on my own and this PR will fix it!

@daviesgeek
Copy link
Collaborator

Awesome thanks. I need to test this a little more, then I'll merge it before the next release

@dokkis
Copy link
Author

dokkis commented Apr 13, 2016

Good :) Take the required time to review and test my PR ;)

@dokkis
Copy link
Author

dokkis commented May 4, 2016

any news on this PR?

@daviesgeek
Copy link
Collaborator

Sorry, life's been a bit crazy. I was trying to test this with a PHP server, but I was unable to ever get Restangular to save the Etag header to the element. Can you confirm that this is the case? Or am I completely misunderstanding the way this works?

@dokkis
Copy link
Author

dokkis commented May 12, 2016

As you can see from the tests and test description it should assign the value of ETag header inside the object, my PR fix the fact that the etag object can be a complex object, example _etag.$oid. The implementation is the same made to support complex object on the id field.

"testing ETag header tests that the ETag header is correctly retrieved by the response headers andits value is injected into the object (in the _etag attribute in the first test and _etag.$oid attribute in the second one). The first one was supported before, infact it's a regression test, the second test shows that with my improvements now Restangular can fill also complex object). The support is really similar to the 'id' field, I just made the things the same as for the id field.

@daviesgeek daviesgeek added this to the 1.5.3 milestone Jun 16, 2016
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.

2 participants