Only search locally for Community::read_from_name and similar (ref #698) #110

nutomic · 2020-10-02T12:23:35Z

nutomic commented

2020-10-02 12:23:35 +00:00

I was going through #698 and wanted to change the json endpoints to only return local objects. Then I noticed that the current implementation of Community::read_from_name and User::find_by_email_or_username doesnt make sense. If a remote user with an identical name exists, then these methods could randomly return the remote user instead of the local one. In fact these methods should only be used for local objects, as remote ones should always be searched by their apub ID.

For post/comment the current implementation makes more sense, as the numeric IDs are unique. So I just added checks in the http handlers.

This might be a problem with RSS feeds as they were previously working with remote users/communities as well. At least in theory, because the frontend shows the feed URL for remote users/communities as /feeds/u/undefined.xml?sort=New. So that should instead link to the feed on the remote instance. Later the feed URL should also be federated (added that to #698).

Edit: federation tests are passing btw

I was going through #698 and wanted to change the json endpoints to only return local objects. Then I noticed that the current implementation of Community::read_from_name and User::find_by_email_or_username doesnt make sense. If a remote user with an identical name exists, then these methods could randomly return the remote user instead of the local one. In fact these methods should only be used for local objects, as remote ones should always be searched by their apub ID. For post/comment the current implementation makes more sense, as the numeric IDs are unique. So I just added checks in the http handlers. This might be a problem with RSS feeds as they were previously working with remote users/communities as well. At least in theory, because the frontend shows the feed URL for remote users/communities as `/feeds/u/undefined.xml?sort=New`. So that should instead link to the feed on the remote instance. Later the feed URL should also be federated (added that to #698). Edit: federation tests are passing btw

dessalines commented

2020-10-05 15:19:01 +00:00

This might be a problem with RSS feeds as they were previously working with remote users/communities as well. At least in theory, because the frontend shows the feed URL for remote users/communities as /feeds/u/undefined.xml?sort=New. So that should instead link to the feed on the remote instance. Later the feed URL should also be federated (added that to #698).

I'll make an issue to re-wire the RSS feeds on the front end for non-local communities to use the actor id / original community rss.

> This might be a problem with RSS feeds as they were previously working with remote users/communities as well. At least in theory, because the frontend shows the feed URL for remote users/communities as /feeds/u/undefined.xml?sort=New. So that should instead link to the feed on the remote instance. Later the feed URL should also be federated (added that to #698). I'll make an issue to re-wire the RSS feeds on the front end for non-local communities to use the actor id / original community rss.

dessalines referenced this issue from a commit

2020-10-05 15:25:10 +00:00

Merge pull request 'Only search locally for Community::read_from_name and similar (ref #698)' (#110) from read-only-local into main Reviewed-on: https://yerbamate.dev/LemmyNet/lemmy/pulls/110

dessalines merged commit 75ace1192a into main

2020-10-05 15:25:11 +00:00

Sign in to join this conversation.

No reviewers

No Label

No Milestone

No Assignees

2 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: LemmyNet/lemmy#110