Parse-SDK-Flutter icon indicating copy to clipboard operation
Parse-SDK-Flutter copied to clipboard

fix: Arrays being set as empty when calling clearUnsavedChanges()

Open nabos440 opened this issue 8 months ago • 12 comments

Pull Request

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.

nabos440 avatar Mar 21 '25 23:03 nabos440

🚀 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: @.***>

nabos440 avatar Mar 22 '25 05:03 nabos440

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

mbfakourii avatar Mar 22 '25 07:03 mbfakourii

I have seen the example. I can't seem to figure it out, I'm still a rookie developer. Can you do it yourself?

nabos440 avatar Mar 22 '25 14:03 nabos440

When will it be merged?

nabos440 avatar Mar 24 '25 00:03 nabos440

@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.

mtrezza avatar Mar 24 '25 01:03 mtrezza

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.

codecov[bot] avatar Mar 24 '25 01:03 codecov[bot]

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

nabos440 avatar Mar 24 '25 02:03 nabos440

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?

mtrezza avatar Mar 27 '25 19:03 mtrezza

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?

Yes, this can be checked with the dart format --output=none --set-exit-if-changed packages/dart command.

mbfakourii avatar Apr 19 '25 09:04 mbfakourii

But what needs to change in the command so that it outputs the issue?

mtrezza avatar Apr 19 '25 14:04 mtrezza

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.

mbfakourii avatar Jun 27 '25 05:06 mbfakourii