Post remove delete federation outbound fix0 #3613
Conversation
RocketDerp
requested review from
Nutomic,
dessalines and
phiresky
as code owners
|
I dont understand, the linked issues are about post deletions not being federated. Yet this PR doesnt change anything related to federation, but changes API responses. Anyway we have test cases to ensure that deletes are federating, so it should work. But its always possible that federated actions dont get delivered, eg because a server is overloaded or because of connection issues. |
This comment was marked as abuse.
This comment was marked as abuse.
This comment was marked as abuse.
This comment was marked as abuse.
Nutomic
force-pushed
the
post_remove_delete_federation_fix0
branch
from
620f036
to
cb91650
Compare
|
Ah I understand it now. The SendActivity trait relies on the return value of the api call to send the activity. However delete/remove post api calls currently dont return the PostView unless the user is an admin (which is probably the reason that tests are still passing). This PR is a good short-term fix which we should include in 0.18.3. Later the problem will get fixed with #3596 which gets rid of SendActivity, and has explicit function calls for each activity send. @RocketDerp You need to run Edit: I wonder if the same problem happens with other actions. Delete/remove comment is fine because CommentView::read doesnt filter for removed/deleted at all, not sure if thats intentional. An alternative fix for this problem would be to change PostView::read so that it doesnt filter for removed/deleted in case my_person_id is the post creator. That would actually be a much cleaner solution. |
This comment was marked as abuse.
This comment was marked as abuse.
|
That change wouldnt help as it only changes PostQuery::read, which isnt used in this case. Basically the same changes would have to be applied to PostView::read which is the method used by build_post_response. |
This comment was marked as abuse.
This comment was marked as abuse.
* add new function build_post_response_deleted_allowed * PostDelete uses new function build_post_response_deleted_allowed * RemovePost uses new build_post_response_deleted_allowed function * code comments about mod or admin flag having other use * reformat "cargo +nightly fmt --all"
* add new function build_post_response_deleted_allowed * PostDelete uses new function build_post_response_deleted_allowed * RemovePost uses new build_post_response_deleted_allowed function * code comments about mod or admin flag having other use * reformat "cargo +nightly fmt --all"
Should fix issue #3588 and #3535 that were introduced on June 23 commit dce79b8