FoundKeyGang/FoundKey
FoundKeyGang/FoundKey
7
19
Fork 22
Code Issues 37 Pull requests 4 Releases Activity

backend: implement not forwarding block activities #212

Merged
norm merged 12 commits from non-federating-blocks into main 2022-11-17 21:24:40 +00:00
Conversation 3 Commits 12 Files changed 11 +213 -19
norm commented 2022-10-21 21:22:49 +00:00
Owner
Copy link

Fixes #211

Commits pulled from https://github.com/misskey-dev/misskey/pull/7799

Todo:

  • Make Block sending opt-in, not opt-out. Changed back to federating blocks by default.

Changelog: Added

Fixes https://akkoma.dev/FoundKeyGang/FoundKey/issues/211 Commits pulled from https://github.com/misskey-dev/misskey/pull/7799 Todo: - [x] ~~Make Block sending opt-in, not opt-out.~~ Changed back to federating blocks by default. Changelog: Added
norm changed title from backend: implement not forwarding block activities to WIP: backend: implement not forwarding block activities 2022-10-21 21:24:56 +00:00
norm force-pushed non-federating-blocks from cce6973326 to e6a13c177d 2022-10-21 21:52:43 +00:00 Compare
norm changed title from WIP: backend: implement not forwarding block activities to backend: implement not forwarding block activities 2022-10-22 16:35:55 +00:00
norm commented 2022-10-22 16:38:46 +00:00
Author
Owner
Copy link

I suggest squashing this PR when merging, with Co-authored-by trailers of course.

I suggest squashing this PR when merging, with `Co-authored-by` trailers of course.
toast commented 2022-11-09 18:03:17 +00:00
Owner
Copy link
  1. Do we send a reject follow in place of a block? Otherwise remote users will keep being able to read private statuses.
  2. Can we make the default enabled?

I know the spec says "SHOULD NOT" but that's fairly permissive, and there are many good reasons to send them out (that have been discussed previously in IRC).
Otherwise this is fine imo - letting users pick is all well and good, it just should be on by default for people that do not care (rather than off, which leads to some fairly bad scenarios).

1. Do we send a reject follow in place of a block? Otherwise remote users will keep being able to read private statuses. 2. Can we make the default enabled? I know the spec says "SHOULD NOT" but that's fairly permissive, and there are many good reasons to send them out (that have been discussed previously in IRC). Otherwise this is fine imo - letting users pick is all well and good, it just should be on by default for people that do not care (rather than off, which leads to some fairly bad scenarios).
norm force-pushed non-federating-blocks from 2a42a41147 to 30a192bdf1 2022-11-09 19:22:02 +00:00 Compare
norm force-pushed non-federating-blocks from 30a192bdf1 to b4a83bea38 2022-11-09 19:23:17 +00:00 Compare
norm commented 2022-11-09 21:53:21 +00:00
Author
Owner
Copy link

Ok, I should have those concerns addressed.

Ok, I should have those concerns addressed.
Johann150 requested changes 2022-11-09 22:55:48 +00:00
packages/backend/migration/1631880003000-user-block-federation.ts Outdated
@ -0,0 +1,14 @@
import {MigrationInterface, QueryRunner} from "typeorm";
Johann150 commented 2022-11-09 22:54:55 +00:00
Owner
Copy link

I think this migration should be adapted to how the other migrations look. I.e. plain javascript, therefore no need for this import.

There should be no type annotations and the file extension should be .js.

I think this migration should be adapted to how the other migrations look. I.e. plain javascript, therefore no need for this import. There should be no type annotations and the file extension should be `.js`.
norm marked this conversation as resolved
Johann150 reviewed 2022-11-12 20:56:42 +00:00
packages/backend/src/services/blocking/delete.ts Outdated
@ -28,3 +28,3 @@
// deliver if remote bloking
if (Users.isLocalUser(blocker) && Users.isRemoteUser(blockee)) {
if (Users.isLocalUser(blocker) && Users.isRemoteUser(blockee) && blocker.federateBlocks) {
Johann150 commented 2022-11-12 20:56:29 +00:00
Owner
Copy link

This could lead to problems if the block was federated but the user later decides blocks should not be federated and then the unblock is not federated.

Since I think unblocks must signify some level of trust it should always be okay to federate them. Or otherwise, whether the block was federated or not should be stored in the database.

This could lead to problems if the block was federated but the user later decides blocks should not be federated and then the unblock is not federated. Since I think unblocks must signify some level of trust it should always be okay to federate them. Or otherwise, whether the block was federated or not should be stored in the database.
norm marked this conversation as resolved
Johann150 reviewed 2022-11-16 20:27:57 +00:00
packages/backend/src/models/repositories/user.ts Outdated
@ -335,6 +335,7 @@ export const UserRepository = db.getRepository(User).extend({
isLocked: user.isLocked,
isSilenced: user.isSilenced || falsy,
isSuspended: user.isSuspended || falsy,
federateBlocks: user!.federateBlocks,
Johann150 commented 2022-11-16 20:27:57 +00:00
Owner
Copy link

I think it might be a better idea to move this to the opts.detail && isMe part further down. I.e. not show this publicly since we've also stopped showing blocked instances publicly.

I think it might be a better idea to move this to the `opts.detail && isMe` part further down. I.e. not show this publicly since we've also stopped showing blocked instances publicly.
norm marked this conversation as resolved
norm force-pushed non-federating-blocks from 16c9647597 to 557885a720 2022-11-16 22:25:30 +00:00 Compare
norm force-pushed non-federating-blocks from 557885a720 to 667d791c85 2022-11-16 22:29:32 +00:00 Compare
Johann150 approved these changes 2022-11-17 20:06:35 +00:00
norm merged commit 71b3b5a60c into main 2022-11-17 21:24:40 +00:00
norm deleted branch non-federating-blocks 2022-11-17 21:24:41 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Johann150
Labels
Clear labels   
feature

task that proposes new feature(s)

  
fix

task to fix behavioral problems in the FoundKey codebase

  
upkeep

task that should make FoundKey smaller and more maintainable

No labels
feature
fix
upkeep
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: FoundKeyGang/FoundKey#212
No description provided.
Delete branch "non-federating-blocks"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?