8000 Feat: Add enable param in Database and Collection by 2002Bishwajeet · Pull Request #5553 · appwrite/appwrite · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Feat: Add enable param in Database and Collection #5553

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
Merged
merged 17 commits into from
May 29, 2023
Merged

Feat: Add enable param in Database and Collection #5553

merged 17 commits into from
May 29, 2023

Conversation

2002Bishwajeet
Copy link
Contributor
@2002Bishwajeet 2002Bishwajeet commented May 18, 2023 •
edited
Loading

What does this PR do?

Adds enabled parameter while creating/updating Database and Collection

Test Plan

Related PRs and Issues

nil

Checklist

  • Have you read the Contributing Guidelines on issues?
  • If the PR includes a change to an API's metadata (desc, label, params, etc.), does it also include updated API specs and example docs?

@2002Bishwajeet
Copy link
Contributor Author

cc @abnegate

@abnegate abnegate self-requested a review May 19, 2023 03:43
abnegate
abnegate reviewed May 19, 2023
View reviewed changes
Copy link
Member
@abnegate abnegate left a comment

Choose a reason for hiding this comment

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

Looks good 👍 just some small formatting comments. I've re-run the CI to check tests again too (the ones that failed are flaky and unrelated to your changes)

@2002Bishwajeet 2002Bishwajeet requested a review from abnegate May 19, 2023 07:25
abnegate
abnegate reviewed May 19, 2023
View reviewed changes
Copy link
Member
@abnegate abnegate left a comment

Choose a reason for hiding this comment

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

Please add some more tests to cover all other updates endpoints too 🙏

@2002Bishwajeet
chore: add more test coverage
< 8000 div class="pr-1 d-md-inline-block d-none">
7fe1f56
@2002Bishwajeet
Add more Tests
45f5d93
@2002Bishwajeet 2002Bishwajeet requested a review from abnegate May 22, 2023 14:47
abnegate
abnegate reviewed May 23, 2023
View reviewed changes
Copy link
Member
@abnegate abnegate left a comment

Choose a reason for hiding this comment

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

Nice work, just a couple of formatting issues

2002Bishwajeet and others added 2 commits May 23, 2023 13:30
@2002Bishwajeet @abnegate
Update tests/e2e/Services/Databases/DatabasesCustomServerTest.php
6e5ee27 
Co-authored-by: Jake Barnby <jakeb994@gmail.com>
@2002Bishwajeet @abnegate
Update tests/e2e/Services/Databases/DatabasesCustomServerTest.php
4964b5c 
Co-authored-by: Jake Barnby <jakeb994@gmail.com>
@abnegate
Copy link
Member

Just realized we also need to add the checks to ensure that only the console can access these routes if a database is disabled, you can see an example of the same check for storage buckets here.

We will need this check for all of the create/read/update/delete routes for collections and documents.

Send me a DM if you need any clarification 😄

@2002Bishwajeet 2002Bishwajeet requested a review from abnegate May 24, 2023 10:10
abnegate
abnegate reviewed May 24, 2023
View reviewed changes
Copy link
Member
@abnegate abnegate left a comment

Choose a reason for hiding this comment

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

Nice, please also add tests to DatabasesConsoleClientTest that ensures that the console can still access these routes for a disabled database

@2002Bishwajeet 2002Bishwajeet requested a review from abnegate May 24, 2023 17:42
abnegate
abnegate reviewed May 26, 2023
View reviewed changes
@2002Bishwajeet 2002Bishwajeet requested a review from abnegate May 26, 2023 12:26
abnegate
abnegate reviewed May 29, 2023
View reviewed changes
Copy link
Member
@abnegate abnegate left a comment

Choose a reason for hiding this comment

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

Some small formatting issues and we're good! Make sure to run composer format or docker run -itv $PWD:/app composer format to auto-fix any issues possible 👍

@2002Bishwajeet 2002Bishwajeet requested a review from abnegate May 29, 2023 07:45
abnegate
abnegate reviewed May 29, 2023
View reviewed changes
@abnegate abnegate merged commit f45905e into appwrite:master May 29, 2023
@2002Bishwajeet 2002Bishwajeet deleted the feat-add-enable-param-db branch May 29, 2023 11:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abnegate abnegate abnegate approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@2002Bishwajeet @abnegate
0