Skip to content

Conversation

dplewis
Copy link
Member

@dplewis dplewis commented Aug 21, 2019

Returns a promise boolean if the object exists on the server

See parse-community/parse-server#5950

@codecov
Copy link

codecov bot commented Aug 21, 2019

Codecov Report

Merging #898 into master will decrease coverage by 0.04%.
The diff coverage is 70%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #898      +/-   ##
==========================================
- Coverage    92.1%   92.05%   -0.05%     
==========================================
  Files          54       54              
  Lines        5026     5036      +10     
  Branches     1127     1129       +2     
==========================================
+ Hits         4629     4636       +7     
- Misses        397      400       +3
Impacted Files Coverage Δ
src/ParseObject.js 89.98% <70%> (-0.23%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3af83ea...79e2b52. Read the comment docs.

@dplewis dplewis requested review from davimacedo and TomWFox August 21, 2019 20:47
Copy link
Member

@davimacedo davimacedo left a comment

Choose a reason for hiding this comment

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

Good job! I have two comments.

await this.fetch(options);
return true;
} catch (e) {
return false;
Copy link
Member

Choose a reason for hiding this comment

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

I think we should check e.code === 101 and only return false in this case. Otherwise throw e. I am afraid that a network issue could be interpreted as an inexistent object.

Copy link
Member

@davimacedo davimacedo Aug 21, 2019

Choose a reason for hiding this comment

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

I was thinking here again. Maybe do a query by id would be better here instead of the fetch? I am not sure if the developer, when calling an exists() function, is expecting the object data to be updated.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thats a good point.

@dplewis dplewis requested a review from davimacedo August 22, 2019 01:01
Copy link
Member

@davimacedo davimacedo left a comment

Choose a reason for hiding this comment

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

LGTM!

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