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.

Sign up for GitHub

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

Jump to bottom

feat(firebase_database)!: add web support to the plugin #6952

Merged
merged 49 commits into from
Sep 14, 2021
Merged

feat(firebase_database)!: add web support to the plugin #6952

merged 49 commits into from
Sep 14, 2021

Conversation

deepak786
Copy link
Contributor

@deepak786 deepak786 commented Sep 5, 2021 •
edited
Loading

Description

This PR adds web support to the plugin firebase_database.

Related Issues

Fixes flutter/flutter#48542

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]).
This will ensure a smooth and quick review process. Updating the pubspec.yaml and changelogs is not required.

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • My PR includes unit or integration tests for all changed/updated/fixed behaviors (See [Contributor Guide]).
  • All existing and new tests are passing.
  • I updated/added relevant documentation (doc comments with ///).
  • The analyzer (melos run analyze) does not report any problems on my PR.
  • I read and followed the [Flutter Style Guide].
  • I signed the [CLA].
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • Yes, this is a breaking change.
  • No, this is not a breaking change.

@google-cla google-cla bot added the cla: yes label Sep 5, 2021
@deepak786 deepak786 changed the title feat(firebase_database): added web support to the plugin feat(firebase_database): add web support to the plugin Sep 5, 2021
deepak786
deepak786 commented Sep 6, 2021
View reviewed changes
Copy link
Contributor Author

@deepak786 deepak786 left a comment

Choose a reason for hiding this comment

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

@Salakar @lesnitsky @russellwheatley

Following things are pending:

  1. Unit Test cases for firebase_database_web.
  2. Following Integeration tests are failing:
    • runTransaction. The reason is in the below comments.
    • DataSnapshot supports null childKeys for maps. (This test also uses runTransaction so thats why it is failing.)
    • correct order returned from query. I think the Firebase-Dart library is not returning the database query results in correct order. Need to check.

packages/firebase_database/firebase_database_web/lib/src/database_reference_web.dart Outdated Show resolved Hide resolved
packages/firebase_database/firebase_database_web/lib/src/database_reference_web.dart Show resolved Hide resolved
packages/firebase_database/firebase_database_web/test/firebase_database_web_test.dart Outdated Show resolved Hide resolved
@SpajicM
Copy link

SpajicM commented Sep 6, 2021 •
edited
Loading

  • correct order returned from query. I think the Firebase-Dart library is not returning the database query results in correct order. Need to check.

#6822
flutter/flutter#19389

@russellwheatley russellwheatley mentioned this pull request Sep 7, 2021
Closed
13 tasks
@deepak786 deepak786 marked this pull request as ready for review September 7, 2021 20:06
@deepak786 deepak786 marked this pull request as draft September 7, 2021 20:06
@deepak786
Copy link
Contributor Author

@googlebot I consent

@google-cla
Copy link

google-cla bot commented Sep 13, 2021

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

1 similar comment
@google-cla
Copy link

google-cla bot commented Sep 13, 2021

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@Salakar
Copy link
Member

Salakar commented Sep 14, 2021

@lesnitsky I think you may need to give bot consent too

@lesnitsky
Copy link
Member

@googlebot I consent

@google-cla google-cla bot added cla: yes and removed cla: no labels Sep 14, 2021
deepak786
deepak786 commented Sep 14, 2021
View reviewed changes
Copy link
Contributor Author

@deepak786 deepak786 left a comment

Choose a reason for hiding this comment

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

is it not required?

@Salakar Salakar removed the request for review from kroikie September 14, 2021 11:39
@deepak786
Copy link
Contributor Author

I think only the below is pending:
test correct order returned from query is failing.

@lesnitsky
Copy link
Member

tests pass (locally, at least 😅)

@Salakar Salakar merged commit 4805599 into firebase:master Sep 14, 2021
@deepak786 deepak786 mentioned this pull request Sep 14, 2021
Closed
@firebase firebase locked and limited conversation to collaborators Oct 15, 2021
@deepak786 deepak786 deleted the firebase_database_web branch December 10, 2021 16:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lesnitsky lesnitsky lesnitsky left review comments

@Salakar Salakar Salakar approved these changes

Assignees

@lesnitsky lesnitsky

Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[firebase_database] Add web support to plugin
4 participants
@deepak786 @SpajicM @lesnitsky @Salakar