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: refactor pinning authorization logic to use user_tag table #1389

Merged
merged 3 commits into from
Mar 16, 2022
Merged

feat: refactor pinning authorization logic to use user_tag table #1389

merged 3 commits into from
Mar 16, 2022

Conversation

e-schneid
Copy link
Contributor

Closes #1381

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Feb 16, 2022 •
edited
Loading

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 9f73d96
Status: ✅  Deploy successful!
Preview URL: https://99d37e2e.nft-storage-1at.pages.dev

View logs

@e-schneid e-schneid force-pushed the feat/issue-1381-pinning-authorization-via-db branch 2 times, most recently from e4349a8 to dfd253e Compare February 16, 2022 20:11
@JeffLowe JeffLowe assigned ghost Feb 17, 2022
@ghost ghost force-pushed the feat/issue-1381-pinning-authorization-via-db branch 3 times, most recently from ecbc3ae to c7e2d23 Compare February 18, 2022 17:35
@ghost ghost requested a review from alanshaw February 19, 2022 04:57
@e-schneid e-schneid force-pushed the feat/issue-1381-pinning-authorization-via-db branch from 99a7eef to 9f73d96 Compare March 8, 2022 18:58
@JeffLowe JeffLowe mentioned this pull request Mar 9, 2022
Closed
3 tasks
@hugomrdias hugomrdias merged commit b5e99ec into main Mar 16, 2022
@hugomrdias hugomrdias deleted the feat/issue-1381-pinning-authorization-via-db branch March 16, 2022 11:16
@alanshaw
Copy link
Contributor

@e-schneid @trigramdev9 it looks like this PR has broken the build in main - any chance you can take a look?

ghost pushed a commit that referenced this pull request Mar 17, 2022
@jsdevel
Attempting to fix build after #1389 was merged without it being updat…
2145a37 
…ed to the latest from main
ghost pushed a commit that referenced this pull request Mar 17, 2022
@jsdevel
Attempting to fix build after #1389 was merged without it being updat…
64772f4 
…ed to the latest from main
ghost pushed a commit that referenced this pull request Mar 17, 2022
@jsdevel
Attempting to fix build after #1389 was merged without it being updat…
ac17965 
…ed to the latest from main

* Some cleaning of PSA_ALLOW references too
ghost pushed a commit that referenced this pull request Mar 17, 2022
@jsdevel
Attempting to fix build after #1389 was merged without it being updat…
500503c 
…ed to the latest from main

* Some cleaning of PSA_ALLOW references too
ghost pushed a commit that referenced this pull request Mar 17, 2022
@jsdevel
Attempting to fix build after #1389 was merged without it being updat…
c3b93f8 
…ed to the latest from main

* Some cleaning of PSA_ALLOW references too
ghost pushed a commit that referenced this pull request Mar 17, 2022
@jsdevel
Attempting to fix build after #1389 was merged without it being updat…
cf3d65e 
…ed to the latest from main

* Some cleaning of PSA_ALLOW references too
ghost pushed a commit that referenced this pull request Mar 17, 2022
@jsdevel
Attempting to fix build after #1389 was merged without it being updat…
f3f8b22 
…ed to the latest from main

* Some cleaning of PSA_ALLOW references too
gobengo
gobengo reviewed Mar 17, 2022
View reviewed changes
packages/api/test/scripts/helpers.js
value: 'false',
reason: '',
inserted_at: '2/22/2022',
})
Copy link
Contributor

Choose a reason for hiding this comment

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

Wondering: Do these two writes happen in a transaction? If the first succeds and the second throws, is that a valid db state?

ghost pushed a commit that referenced this pull request Mar 17, 2022
@jsdevel
fix: Refactor pinning authorization logic to use user_tag table
52f610e 
* see #1389 #1381
* Adding a missing comma.
* Filtering tags on deleted_at is null.
* Removing some PSA_ALLOW references.
@ghost ghost mentioned this pull request Mar 17, 2022
Merged
@github-actions github-actions bot mentioned this pull request Mar 17, 2022
Merged
the-simian pushed a commit that referenced this pull request Mar 17, 2022
fix: Refactor pinning authorization logic to use user_tag table (#1654)
043049e 
* see #1389 #1381
* Adding a missing comma.
* Filtering tags on deleted_at is null.
* Removing some PSA_ALLOW references.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gobengo gobengo gobengo left review comments

@hugomrdias hugomrdias hugomrdias approved these changes

@alanshaw alanshaw Awaiting requested review from alanshaw

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

Successfully merging this pull request may close these issues.

Refactor pinning authorization logic away from global CloudFlare worker variable to user_tag table
5 participants
@e-schneid @alanshaw @gobengo @hugomrdias @jsdevel