Skip to content

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

Merged
Salakar merged 49 commits into
firebase:masterfrom
deepak786:firebase_database_web
Sep 14, 2021
Merged

feat(firebase_database)!: add web support to the plugin #6952
Salakar merged 49 commits into
firebase:masterfrom
deepak786:firebase_database_web

Conversation

@deepak786

@deepak786 deepak786 commented Sep 5, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

@deepak786 deepak786 left a comment

Copy link
Copy Markdown
Contributor Author

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.

Comment thread packages/firebase_database/firebase_database_web/lib/src/database_reference_web.dart Outdated
Comment thread packages/firebase_database/firebase_database_web/lib/src/database_reference_web.dart
Comment thread packages/firebase_database/firebase_database_web/test/firebase_database_web_test.dart Outdated
@SpajicM

SpajicM commented Sep 6, 2021 •
edited
Loading

Copy link
Copy Markdown
  • 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

deepak786
deepak786 commented Sep 7, 2021
View reviewed changes
Comment thread packages/firebase_database/firebase_database_web/lib/firebase_database_web.dart Outdated
@russellwheatley russellwheatley mentioned this pull request Sep 7, 2021
Closed
13 tasks
lesnitsky
lesnitsky reviewed Sep 7, 2021
View reviewed changes
Comment thread packages/firebase_database/firebase_database_web/lib/firebase_database_web.dart Outdated
@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
deepak786 commented Sep 8, 2021
View reviewed changes
Comment thread packages/firebase_database/firebase_database_web/lib/firebase_database_web.dart Outdated
@deepak786

deepak786 commented Sep 13, 2021

Copy link
Copy Markdown
Contributor Author

@googlebot I consent

@google-cla

google-cla Bot commented Sep 13, 2021

Copy link
Copy Markdown

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

google-cla Bot commented Sep 13, 2021

Copy link
Copy Markdown

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

Salakar commented Sep 14, 2021

Copy link
Copy Markdown
Contributor

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

@lesnitsky

lesnitsky commented Sep 14, 2021

Copy link
Copy Markdown
Contributor

@googlebot I consent

@google-cla google-cla Bot added cla: yes and removed cla: no labels Sep 14, 2021
@Salakar
Salakar requested a review from kroikie as a code owner September 14, 2021 11:17
deepak786
deepak786 commented Sep 14, 2021
View reviewed changes

@deepak786 deepak786 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

is it not required?

deepak786
deepak786 commented Sep 14, 2021
View reviewed changes
Comment thread packages/firebase_database/firebase_database_web/test/firebase_database_web_test.dart Outdated
@Salakar
Salakar removed the request for review from kroikie September 14, 2021 11:39
@deepak786

deepak786 commented Sep 14, 2021

Copy link
Copy Markdown
Contributor Author

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

@lesnitsky

lesnitsky commented Sep 14, 2021

Copy link
Copy Markdown
Contributor

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

2 more reviewers
@lesnitsky lesnitsky lesnitsky left review comments
@Salakar Salakar Salakar approved these changes
Reviewers whose approvals may not affect merge requirements

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