Parse-SDK-Flutter
Parse-SDK-Flutter copied to clipboard
fix: Arrays being set as empty when calling clearUnsavedChanges()
Pull Request
- Report security issues confidentially.
- Any contribution is under this license.
- Link this pull request to an issue.
Issue
Closes: https://github.com/parse-community/Parse-SDK-Flutter/issues/1038
Approach
Added null safety check before clearing savedArray and setting to _estimatedArrayBeforeSaving in the onSaved() function of Parse Array class
This fixes the issue because the _handleSingleResult() function which handles the response for a single parse object in the _ParseResponseBuilder was calling _notifyChildrenAboutSave(); so when save() was called on any object it called _notifyChildrenAboutSaving(); first and then in the response when _notifyChildrenAboutSave(); was called, the _estimatedArrayBeforeSaving had values and _savedArray was not set as empty.
But in case of other queries where single object was returned like getUpdatedUser() or fetch() on any object it just called _notifyChildrenAboutSave(); in the _handleSingleResult() function which resulted in _savedArray being set as empty since _estimatedArrayBeforeSaving was null. I just added the null safety check before setting the _savedArray to _estimatedArrayBeforeSaving so now the savedArray is never returned empty in the response and the issue is resolved.
🚀 Thanks for opening this pull request!
I will reformat the title to use the proper commit message syntax.
How to do that?
On Sat, Mar 22, 2025, 10:22 AM Mohammad Bagher Fakouri < @.***> wrote:
@.**** requested changes on this pull request.
Looks good. Could you add a test ?
— Reply to this email directly, view it on GitHub https://github.com/parse-community/Parse-SDK-Flutter/pull/1039#pullrequestreview-2707827500, or unsubscribe https://github.com/notifications/unsubscribe-auth/AY6HMS7WCHAMJBEKI3GLEKT2VTXRZAVCNFSM6AAAAABZRD7BXCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDOMBXHAZDONJQGA . You are receiving this because you authored the thread.Message ID: @.***>
How to do that? … On Sat, Mar 22, 2025, 10:22 AM Mohammad Bagher Fakouri < @.> wrote: @.* requested changes on this pull request. Looks good. Could you add a test ? — Reply to this email directly, view it on GitHub <#1039 (review)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AY6HMS7WCHAMJBEKI3GLEKT2VTXRZAVCNFSM6AAAAABZRD7BXCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDOMBXHAZDONJQGA . You are receiving this because you authored the thread.Message ID: @.***>
Please check the examples in the link below:
https://github.com/parse-community/Parse-SDK-Flutter/tree/master/packages/dart/test
I have seen the example. I can't seem to figure it out, I'm still a rookie developer. Can you do it yourself?
When will it be merged?
@nabos440 To add a test, just clone an existing test from the packages/dart/test directory. If there's a file that matches the topic of the issue, then just add the test there, otherwise you can create a new file. If you look at the tests there, you should get a sense of how a test is written. A test should fail before your fix and pass after your fix. That way you can verify that the bug has ben fixed correctly, and that it won't be re-introduced in the future. Once you've added the test and committed, it will run in the CI here. Also, I've started the CI for your current change and it failed, looks like a lint issue. You can see how to fix them by opening a job log below.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 9.53%. Comparing base (
c388545) to head (2e490c4). Report is 13 commits behind head on master.
:exclamation: There is a different number of reports uploaded between BASE (c388545) and HEAD (2e490c4). Click for more details.
HEAD has 3 uploads less than BASE
Flag BASE (c388545) HEAD (2e490c4) 8 5
Additional details and impacted files
@@ Coverage Diff @@
## master #1039 +/- ##
==========================================
- Coverage 43.37% 9.53% -33.84%
==========================================
Files 61 8 -53
Lines 3463 325 -3138
==========================================
- Hits 1502 31 -1471
+ Misses 1961 294 -1667
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
They are failing because of codcov report upload failure and lint issue for which there is no detail as to what is causing the failure
If you run dart format locally you should see what's causing it. Unfortunately it's not visible with more details in the CI at the moment, not sure whether the command dart format --output=none --set-exit-if-changed packages/dart allows to change that, @mbfakourii?
If you run
dart formatlocally you should see what's causing it. Unfortunately it's not visible with more details in the CI at the moment, not sure whether the commanddart format --output=none --set-exit-if-changed packages/dartallows to change that, @mbfakourii?
Yes, this can be checked with the dart format --output=none --set-exit-if-changed packages/dart command.
But what needs to change in the command so that it outputs the issue?
But what needs to change in the command so that it outputs the issue?
But what needs to change in the command so that it outputs the issue?
The problem is visible in ci errors, no need to add any code.