parse-community / Parse-SDK-Flutter

The Dart/Flutter SDK for Parse Platform
https://parseplatform.org
Apache License 2.0
573 stars 188 forks source link

feat: Add `installationId` in LiveQuery connect #976

Closed mbfakourii closed 9 months ago

mbfakourii commented 9 months ago

Pull Request

Issue

Closes: #975

Approach

add installationId in LiveQuery connect.

Tasks

parse-github-assistant[bot] commented 9 months ago

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

parse-github-assistant[bot] commented 9 months ago

Thanks for opening this pull request!

codecov[bot] commented 9 months ago

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (f757dad) 40.60% compared to head (f2d35bd) 44.12%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #976 +/- ## ========================================== + Coverage 40.60% 44.12% +3.51% ========================================== Files 60 60 Lines 3401 3404 +3 ========================================== + Hits 1381 1502 +121 + Misses 2020 1902 -118 ``` | [Files](https://app.codecov.io/gh/parse-community/Parse-SDK-Flutter/pull/976?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=parse-community) | Coverage Δ | | |---|---|---| | [...ackages/dart/lib/src/network/parse\_live\_query.dart](https://app.codecov.io/gh/parse-community/Parse-SDK-Flutter/pull/976?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=parse-community#diff-cGFja2FnZXMvZGFydC9saWIvc3JjL25ldHdvcmsvcGFyc2VfbGl2ZV9xdWVyeS5kYXJ0) | `50.90% <71.42%> (+50.90%)` | :arrow_up: | ... and [2 files with indirect coverage changes](https://app.codecov.io/gh/parse-community/Parse-SDK-Flutter/pull/976/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=parse-community)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

mbfakourii commented 9 months ago

Could you add a test?

These additional items are in a private function, it is also for socket operation and cannot be tested.

mtrezza commented 9 months ago

Not sure I understand; why do you think it cannot be tested?

mbfakourii commented 9 months ago

Not sure I understand; why do you think it cannot be tested?

Changes in this function. This function is a private function. It cannot be accessed from outside.

mtrezza commented 9 months ago

Is it not possible to access a private function in tests? How are all the LiveQuery features then tested?

mbfakourii commented 9 months ago

If it not possible to access a private function in tests? How are all the LiveQuery features then tested?

There are currently no tests for LiveQuery!

mtrezza commented 9 months ago

Interesting. It should be easily possible to test an internal method. Can't you just reference the internal method directly in the test file, mock channel, execute the method and check whether the connect message has the installationId set?

mbfakourii commented 9 months ago

Interesting. It should be easily possible to test an internal method. Can't you just reference the internal method directly in the test file, mock channel, execute the method and check whether the connect message has the installationId set?

I did not test with mockito, I will try to implement

mbfakourii commented 9 months ago

@mtrezza

Test added

mtrezza commented 9 months ago

Congrats for adding the first live query test!

maravilhosinga commented 2 months ago

This should be optional since, it breaks live query when using lower versions of parse server. Specially for who uses sashido.io

mtrezza commented 2 months ago

@maravilhosinga could you please open a new issue and provide more specifics about what's breaking?