-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
feat: add encrypt param to string attribute #5604
Merged
abnegate
merged 42 commits into
appwrite:cl-1.4.x
from
2002Bishwajeet:feat-add-encrypt-param
Aug 15, 2023
Merged
feat: add encrypt param to string attribute #5604
abnegate
merged 42 commits into
appwrite:cl-1.4.x
from
2002Bishwajeet:feat-add-encrypt-param
Aug 15, 2023
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
abnegate
reviewed
Jun 22, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work
abnegate
approved these changes
Jun 23, 2023
christyjacob4
requested changes
Jul 5, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please address the comments
2002Bishwajeet
changed the title
feat: add encrypt param to attributes
feat: add encrypt param to string attribute
Jul 12, 2023
2002Bishwajeet
commented
Jul 12, 2023
- cache caused stale data in memberships
This commit contains changes in 3 places. - First I changed the placement of an informative comment in the DELETE TEAM controller, and moved it outside of the loop. I did this when Steven pointed out that the behaviour I describe in the comment is for the whole loop. - The second change is the removal of my first test. I was facing quite a few issues with creating users in the test, and ended up using the CREATE TEAM MEMBERSHIP to perform 2 actions at once -> create a new user if one doesn't exist with the provided email, and create a membership for the user. Before this approach, I had quite a bit of code that didn't work, and it seems like I removed some things that weren't supposed to be removed, and didn't change variable names where necessary. Anyway, I figured that the problem has something to do with the user being created on the client side, so I moved the test to the server side. - The new test I implemented does the same thing as my previous failed test, but in more detailed and distinct steps. The test first creates 5 new users inside of a loop, and pushes each new user's ID to an array called 'new_users' if the response is as expected. Then a new team is created. The next step is to create memberships for all 5 users. If all these steps pass, the new team that was just created, is deleted, and we check to make sure the new users have 0 team memberships each. Formatter and linter showed no errors. Tests were successful on localhost.
The CREATE TEAM MEMBERSHIP endpoint requires the email of the user to be added to the team. If the user does not exist in the project, a new user is created with the specified email and added to the team. The first version of the test creates 5 users, and then adds them to the newly created team. This process is more streamlined now, by using the CREATE TEAM MEMBERSHIPS behaviour to create a user on the go and create a membership for them immediately after. I also change the way I add user IDs to the array, by using the shorthand notation instead of the `array_push` function.
Run the composer format and lint commands, which I forgot to run before.
…ail-template-missing-word Added missing word to portuguese email template
…m-memberships-on-user Fixed bug where memberships remained after a team is deleted
…pwrite-id-header-for-CreateFile Validate value of x-appwrite-id header
christyjacob4
approved these changes
Aug 8, 2023
abnegate
force-pushed
the
feat-add-encrypt-param
branch
from
August 14, 2023 23:38
ff4662a
to
4ddb0ce
Compare
abnegate
approved these changes
Aug 15, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Add and optional encrypt param to encrypt attributes
Test Plan
Unit and E2E testing
Related PRs and Issues
Fix #3150
Checklist