Skip to content

Conversation

@Karvel
Copy link

@Karvel Karvel commented Aug 2, 2016

  • Added delete function to API scripts and deprecated remove functions.
  • Added Field tests.
  • Updated Query to use named parameters/objects.
  • Updated Query tests to match new Query functions.

TODO

  • Fix between() refactor in Query script and update respective test.
  • Fix orderBy() refactor in Query script and update respective test.
  • Finish tests for API scripts.
  • Add unit test for delete function in Query.

Karvel added 24 commits July 15, 2016 10:01
…r with a feature deprecated message on using remove
…th a feature deprecated message on using remove
…with a feature deprecated message on using remove
…th a feature deprecated message on using remove
…with a feature deprecated message on using remove
…th a feature deprecated message on using remove
…$`, add depreciation warning when using argument without `$`
…y.js` - orderBy, between, getIntersecting, and getNearest still not passing
@Karvel
Copy link
Author

Karvel commented Aug 3, 2016

Updated to close #10.

@Karvel
Copy link
Author

Karvel commented Aug 11, 2016

Also closes #17.

…ge-Inc/javascript-montage into feature/parity-with-python-wrapper
feat(task): add task endpoint to wrapper
feat(project): add project API endpoint
return this.client.request(`project`);
}

update(name, subdomain) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Karvel: Please sprinkle in a bit of destructuring here.

describe('#istarts()', () => {
it('should equal a filter with a value of $istarts', () => {
let expectedFilter = [['$eq', 'y']];
expect(field.eq('y').filters).to.eql(expectedFilter);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This tests eq() instead of istarts()

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in a4c65e4.

@nick-herrera
Copy link

nick-herrera commented Sep 19, 2016

@dmpayton @Karvel Should Field.contains() be removed as requested in issue #17? The above conversation says that it closes #17, but I don't see how.

@Karvel are the Query.between() and Query.orderBy() refactors you mention in your to-do list complete?

@dmpayton do you want to wait on implementing tests for the API classes (document, file, etc)?

I'd recommend implementing a unit test for Query.delete(). Also, Query.filter() and Query.group() don't have any tests. Finally, the test for Field.istarts() actually tests Field.eq()

@Karvel
Copy link
Author

Karvel commented Sep 19, 2016

@nick-herrera I erroneously thought that 66dae52 addressed #17. I took care of it in a062c1f.

@dmpayton
Copy link

I'm cool with merging this once the 4 TODOs in the original comment are complete.

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.

4 participants