-
Notifications
You must be signed in to change notification settings - Fork 554
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
quote aggs, list quotes #2658
quote aggs, list quotes #2658
Conversation
4972bef
to
82a04b1
Compare
rework schema rework schema add getPostQuotes to api use posts use posts codegen use items instead of quotes codegen add getPostQuotes add quoteCount to response update lexicon for postview increment post ags add quote to post aggs add quote interface oops add quote table migration
fc133e1
to
d468f06
Compare
packages/bsky/src/data-plane/server/db/migrations/20240723T220703655Z-quotes.ts
Show resolved
Hide resolved
packages/bsky/src/data-plane/server/db/migrations/20240723T220703655Z-quotes.ts
Outdated
Show resolved
Hide resolved
…703655Z-quotes.ts Co-authored-by: devin ivy <devinivy@gmail.com>
0d2e2e0
to
b75f08d
Compare
@@ -304,6 +339,7 @@ const deleteFn = async ( | |||
.executeTakeFirst(), | |||
db.deleteFrom('feed_item').where('postUri', '=', uriStr).executeTakeFirst(), | |||
]) | |||
await db.deleteFrom('quote').where('subject', '=', uriStr).execute() |
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.
I think this is the right place to stick this, deleting all the quotes for the post once we delete it.
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.
There's another one of these below. I think we can nix this one.
await db.deleteFrom('quote').where('subject', '=', uriStr).execute() |
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.
okay looks like w/o this the test fails. let me actually pull down the code and look while i fix up that test.
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.
Ohh you know, I think I had it wrong. The quote gets deleted if either the post or the quoted post get deleted.
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.
ah yep. added this line back now.
Having problems getting |
@haileyok I'm wondering (and I'm probably not seeing it in the code) but shouldn't there also be an optional quoteID in the viewer? That way we can display that the user has already quoted a post. |
@seboslaw I'm thinking that we shouldn't care about the user has already quoted a post or not? If the user makes more than one quote each should show up both in listing and in the quote count. From what I could tell this is also how Twitter works (though maybe there's more to it?) |
@haileyok I just checked. You're right. Twitter also doesn't show it (which I thought it did) and since you can quote multiple times - like you said - it makes no sense to indicate a 'made quote'. 👍 |
6ad65f7
to
b4e6f8d
Compare
Co-authored-by: devin ivy <devinivy@gmail.com>
* origin/main: quote aggs, list quotes (#2658)
No description provided.