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(core): Update QueryPagination page field to default to 0 #1533

Merged
merged 1 commit into from
Apr 18, 2022

Conversation

cshfang
Copy link
Member

@cshfang cshfang commented Apr 16, 2022

Issue #, if available: #842

Description of changes: This PR updates the default constructor for QueryPaginations to supply a default value of 0 for the page field. The Kotlin version of the QueryPaginationBuilder checks to see if the serialized map passed to it contains the page key, but since the serializeAsMap() method always adds the page key, a NullPointerException can arise as a result.

Another fix considered was to update the serializeAsMap() method to only add the page key if page is defined. This would be a reasonable fix but only when coupled with the current QueryPaginationBuilder utilities. Opting for the default value would not require the "is the page key defined check.

  • Also of note - Rather than removing these checks from the utilities, opting to keep them there as they are, in theory, directly usable outside of Flutter method channel context as well.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@cshfang cshfang requested a review from a team as a code owner April 16, 2022 02:24
@codecov-commenter
Copy link

codecov-commenter commented Apr 16, 2022

Codecov Report

Merging #1533 (8aaa54e) into main (ce9c4fa) will not change coverage.
The diff coverage is 0.00%.

@@           Coverage Diff           @@
##             main    #1533   +/-   ##
=======================================
  Coverage   46.47%   46.47%           
=======================================
  Files         262      262           
  Lines       10194    10194           
=======================================
  Hits         4738     4738           
  Misses       5456     5456           
Flag Coverage Δ
android-unit-tests ∅ <ø> (∅)
flutter-unit-tests 36.63% <0.00%> (ø)
ios-unit-tests 85.12% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ify_core/lib/src/types/query/query_pagination.dart 0.00% <0.00%> (ø)

Copy link
Contributor

@dnys1 dnys1 left a comment

Choose a reason for hiding this comment

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

Thanks!

@cshfang cshfang merged commit f9f5d22 into aws-amplify:main Apr 18, 2022
@cshfang cshfang deleted the fix-query-pagination branch April 18, 2022 21:01
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.

4 participants