Closed (fixed)
Project:
Social Post
Version:
8.x-2.x-dev
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
10 Apr 2018 at 08:10 UTC
Updated:
30 May 2019 at 03:34 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
gvsoYes, I was aware of this issue. I haven't had time to completely troubleshoot it. When I tried to fix it, either the list of associated accounts was shown in the administrator page or in the user edit form (and there were issues when trying to delete a linked account), but not in both like if there was a kind of incompatibility in the route path.
Look at this and this.
Thanks for creating this ticket. We should fix this bug in Social Post core.
Comment #3
codebymikey commentedDid some messing around, and commenting out the
provider: 'provider'default entries in the `entity.social_post.collection` and `entity.social_post.delete_form` route allows it to work, I haven't dug into the documentation properly for why that was stopping it from working though.Comment #4
codebymikey commentedRelevant patch
Comment #5
ragasirtahk commentedThis patch fixed the not found error. But, I am still wondering how to add user entities. Not RTBC(ing), waiting for someone else to review as well.
Comment #6
ragasirtahk commented(Edited)
Each user with autopost permission could add their account /user/{user-id}/edit .
Added an account from there and tested the /admin/config/social-api/social-post/twitter/users page.
There were some problems though:
The admin was able to add accounts only for himself both ways (from admin page and from user edit form page). The admin could not add accounts for other users in any way.
The admin could delete their associated account both ways (from admin page and from user edit form page).
The user could not delete their account from /admin/config/social-api/social-post/linkedin/users as well as from /user/{user-id}/edit. Both returned access denied error.
The admin could delete other users' account from /admin/config/social-api/social-post/linkedin/users but not from /user/{user-id}/edit. The admin could delete only its account from /user/{user-id}/edit.
Comment #7
ragasirtahk commentedComment #8
gvsoThe admin shouldn't be able to do this. The only way to associate an account is to authorize the app from the user's Twitter account. Nobody else than the user should know his/her Twitter password.
The user should not be able to delete an account from the admin page. That's a permission set up in .permissions.yml (in Social Post) which is only reserved for accounts with high permissions. Now, if the user can't delete the account from user/{user-id}/edit and it *has* the 'Delete own Social Post user accounts' permission, then that's a bug.
Comment #9
chop commentedIn issue #2936334: Refactor base code the SocialPost entity links were refactored, changing the collection link pattern:
from
"/admin/config/social-api/social-post/{provider}/users"to
"/admin/config/social-api/social-post/users/{provider}"however the corresponding route did not get the same treatment, which has effectively broken the Users page for social post providers that reference the route. This included the Social Post Twitter module. By fixing the Route, so it matches the refactored entity links, I think I've fixed the broken pages.
Works with Social Post Twitter 8.x-2.x
Comment #10
chop commentedPlease review against other dependent modules.
Comment #11
gvsoThe above approach makes all tabs (of all implementers) in the settings page to be highlighted. It seems like that happens because we are trying to use the same route as the
route_nameThis new approach might work. Please, let me know!
Comment #12
chop commentedCan't we share the route, without multiple tabs being highlighting at once? Is that a thing?
What if we reverse the refactor of the collection links path, to put the
{provider}earlier in the path?Can we test reversing the change to the collection links path from
"/admin/config/social-api/social-post/users/{provider}"To the previously used collection links path of
"/admin/config/social-api/social-post/{provider}/users"Does highlighting match then for more than one link that shares the route?
As I don't have an install with more than one Social Post module on - ie. LinkedIn with Twitter - can you try this and see? In theory it should not highlight both, but if you could test, that'd be great. It is worth a try.
It'd just be good to keep the change small and contained to just this module only, without modules having to implement their own route. I think the original idea, to provide a shared collection link and route is a good idea.
Comment #13
gvsoYeah, I also liked the idea of keeping it simple, but it seems like it's not that easy
I tried this as well. I think the issue is that tasks (the tabs) are referring to the same route which is highlighting all the tabs.
However, I was actually thinking of simplifying things a little bit later after refactoring Social Post to include a base controller. To add a method similar to how redirectToProvider works in Social Auth.
Comment #14
gvsoAny updates on this?
Comment #15
gvsoComment #17
gvsoThis is the solution that works best so far. If we find something better, we can just change it again. I don't think it is a big deal since it wouldn't break things so badly.
Thanks everyone!