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

fix: Updating and deleting a ParseObject sends requests even if object ID is null #829

Merged
merged 5 commits into from
Feb 27, 2023

Conversation

Nidal-Bakir
Copy link
Member

@Nidal-Bakir Nidal-Bakir commented Feb 26, 2023

New Pull Request Checklist

Issue Description

Closes: #828

Approach

Use assert() to make sure that the objectId is valid and can be used

TODOs before merging

  • Add tests
  • Add changes to documentation (guides, repository pages, in-code descriptions)
  • A changelog entry

… server even when the objectId property is null
@parse-github-assistant
Copy link

parse-github-assistant bot commented Feb 26, 2023

Thanks for opening this pull request!

  • 🎉 We are excited about your hands-on contribution!

@codecov
Copy link

codecov bot commented Feb 26, 2023

Codecov Report

Base: 15.82% // Head: 15.78% // Decreases project coverage by -0.04% ⚠️

Coverage data is based on head (af779a2) compared to base (9ff9794).
Patch coverage: 0.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #829      +/-   ##
==========================================
- Coverage   15.82%   15.78%   -0.04%     
==========================================
  Files          47       47              
  Lines        2876     2883       +7     
==========================================
  Hits          455      455              
- Misses       2421     2428       +7     
Impacted Files Coverage Δ
packages/dart/lib/src/objects/parse_object.dart 14.95% <0.00%> (-0.47%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@Nidal-Bakir
Copy link
Member Author

Nidal-Bakir commented Feb 26, 2023

I will write the tests for these changes in my other PR.
Because I don't want to create a merge conflict.

When these changes are approved and merged into the master I will pull them to my code base and write tests for them

@Nidal-Bakir Nidal-Bakir changed the title fix: update/delete functions in ParseObject will send requests to the server even when the objectId property is null fix: update/delete functions in ParseObject will send requests even when the objectId is null Feb 26, 2023
@parse-github-assistant
Copy link

I will reformat the title to use the proper commit message syntax.

@parse-github-assistant parse-github-assistant bot changed the title fix: update/delete functions in ParseObject will send requests even when the objectId is null fix: Update/delete functions in ParseObject will send requests even when the objectId is null Feb 26, 2023
@mtrezza mtrezza requested a review from a team February 26, 2023 23:19
mbfakourii
mbfakourii previously approved these changes Feb 27, 2023
Copy link
Member

@mbfakourii mbfakourii left a comment

Choose a reason for hiding this comment

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

The code was tested and looks good

@mtrezza
Copy link
Member

mtrezza commented Feb 27, 2023

Since this is a fix, could you add a change log entry using the title of this PR and bump the version? It would also be great if you could add a test for the bug that is fixed here, so we can prevent this from happening in the future.

@mtrezza mtrezza changed the title fix: Update/delete functions in ParseObject will send requests even when the objectId is null fix: Update/delete functions in ParseObject will send requests even when the object ID is null Feb 27, 2023
@mtrezza mtrezza changed the title fix: Update/delete functions in ParseObject will send requests even when the object ID is null fix: Updating and deleting a ParseObject sends requests even if object ID is null Feb 27, 2023
@Nidal-Bakir
Copy link
Member Author

Nidal-Bakir commented Feb 27, 2023

@mtrezza

It would also be great if you could add a test for the bug that is fixed here, so we can prevent this from happening in the future.

Adding tests now will create a merge conflict on my other PR.

I will add tests for this on my other PR.

I'm adding tests for every function in ParseObject class on my other PR. so adding tests for this is redundant and will make overhead for me because it will result in conflict

@mtrezza
Copy link
Member

mtrezza commented Feb 27, 2023

Got it, thanks for adding the changelog entry, let's wait for the CI and then merge this...

@mtrezza mtrezza merged commit a19ea87 into parse-community:master Feb 27, 2023
@Nidal-Bakir Nidal-Bakir deleted the bug_fix#828 branch February 27, 2023 23:59
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.

The SDK will try to update or delete ParseObject even if the objectId property is Null
3 participants