[feature] Interaction requests client api + settings panel #3215
Conversation
tsmethurst
changed the title
tsmethurst
force-pushed
the
interaction_requests_client_api
branch
from
a36c033 to
29e4726
Compare
internal/db/bundb/migrations/20240809134448_interaction_requests_client_api.go
Show resolved
Hide resolved
internal/db/bundb/migrations/20240809134448_interaction_requests_client_api.go
Outdated
Show resolved
Hide resolved
| ctx context.Context, | ||
| req *gtsmodel.InteractionRequest, | ||
| ) (*gtsmodel.InteractionApproval, gtserror.WithCode) { | ||
| if req.Like == nil { |
There was a problem hiding this comment.
i don't think we need to check for these here. if they were nil but the ID for the object was there as it should be, then the initial GetInteractionRequest() to the database should have returned error in the final stage populating it. Adding these nil checks here makes it seem like this is an (at least somewhat) expected situation. Absolute worst-case it'll just panic on attempting to deref ptr below.
There was a problem hiding this comment.
also if that does happen, it could potentially be a serious issue. a not-found is likely to end up getting masked and we won't end up dealing with it, whereas a panic and unexpected 500 definitely does get our attention :p
There was a problem hiding this comment.
I updated the db functions to be OK with NotFound errors when trying to fetch a Like, Reply, or Announce to populate an interaction request. Especially now that rejections are using the same model as accepts and requests, there's a fair chance that if someone tries to fetch an interaction request from the db, and it's for something that's been rejected, the Like, Reply, or Announce won't be in the database anymore, so it is a semi-expected error. Given that, I think it's sensible to have nil checking here. I've changed it now so that an error not found is returned if the Like (or whatever) can't be found, which should be warning enough I think.
| } | ||
|
|
||
| if !includeLikes && !includeReplies && !includeBoosts { | ||
| const text = "at least one of favourites, replies, or boosts must be true" |
There was a problem hiding this comment.
i think the api response may make a bit more sense if this was must be set
There was a problem hiding this comment.
I'd rather leave it as is, since "must be set" would also suggest you could set them all, but to false.
| TableExpr("? AS ?", bun.Ident("statuses"), bun.Ident("status")). | ||
| Where("? = ?", bun.Ident("status.account_id"), account.ID). | ||
| Where("? IS NOT NULL", bun.Ident("status.pinned_at")). | ||
| Where("NOT ? = ?", bun.Ident("status.pending_approval"), true). |
There was a problem hiding this comment.
is this actually possible for a pinned status?
There was a problem hiding this comment.
Yeah, I think you could create a reply to someone, and then try to pin your reply. Or something daft like that. I suppose we could just make it so that you can't pin pending statuses, instead.
Description
This pull request adds review and rejection / approval of interaction requests via the client API and settings panel.
To support this, requests and rejections are now stored in the database in addition to approvals, as that was just the easiest way of making it possible to page through requests (and, in future, page through approvals and rejections).
The structure of the approvals table has also changed. To make things a bit easier for myself, the current table is just dropped and recreated with the new schema, which means existing approvals will be lost. Since approvals were not in a release yet, I don't think this is a big deal, since only people running snapshots had interaction policies up to now (and therefore stored Accepts), but I welcome feedback on this and can make more of an effort to preserve existing Accepts if we decide it's necessary to do this.
Still draft because I'd still like to do the following:[ ] Clean up any lingering unique indexes from dropped table (if db type = postgres)Not necessary.[ ] Have the interaction filter check if a previous InteractionRejection exists for a fave or boost of a status by a given account, and automatically reject reattempts (so people can't spam interaction requests at you).I'll do this in a separate PR + it might not be necessary or desirable anyway, not sure yet.Part of #3057
Checklist
Please put an x inside each checkbox to indicate that you've read and followed it:
[ ]->[x]If this is a documentation change, only the first checkbox must be filled (you can delete the others if you want).
go fmt ./...andgolangci-lint run.