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

Get and Set Firebase Rules #136

Merged
merged 3 commits into from
Nov 24, 2017
Merged

Conversation

bennyt2
Copy link
Contributor

@bennyt2 bennyt2 commented Nov 24, 2017

Get and Set Firebase Rules via the REST API, as noted in this documentation: https://firebase.google.com/docs/database/rest/app-management

This is a proposed fix for Issue #132.

@jeromegamez
Copy link
Member

Ooooh, this looks nice, I like it a lot already. As soon as I get back to my computer, I will check it out and play it through once.

Would you mind adding a small section to the docs as well? (The incoming merge doesn't depend on it :))

The top of this document sets $database = $firebase->getDatabase().
These docs have been changed to use the $database variable to
maintain consistency with other documentation on this page.
@bennyt2
Copy link
Contributor Author

bennyt2 commented Nov 24, 2017

Docs added. I think I did them right. I had trouble installing Sphinx on my machine, so I wasn't able to review the formatting locally. Let me know if they need to be changed or improved.

@jeromegamez
Copy link
Member

Thank you Ben! There are some minor issues, but nothing serious. I'll rewrite the docs a little, and I will also add some try/catch blocks to the code - for example: if the first rules key is missing, the API returns an error, which is currently not catched. It would perhaps make sense to abstract the first field away. So the behaviour will probably change slightly.

Anyways, I will merge your changes and add mine afterwards. Thank you very much!

@jeromegamez jeromegamez merged commit 4f957f1 into kreait:master Nov 24, 2017
@bennyt2
Copy link
Contributor Author

bennyt2 commented Nov 24, 2017

@jeromegamez Thank you for merging this code, and for your speedy and thoughtful replies. Have a great weekend!

@jeromegamez
Copy link
Member

@bennyt2 I have made some (yet unreleased) changes to the code and the documentation, you can see them here:

Commit: c2128d6
Docs: http://firebase-php.readthedocs.io/en/latest/realtime-database.html#database-rules

@bennyt2
Copy link
Contributor Author

bennyt2 commented Nov 25, 2017

@jeromegamez I tested this out in my app and it works great. I like that you made the RuleSet class. Makes it much easier to visualize, and forces people to use Rules correctly. Thank you!!

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.

2 participants