Skip to content

Conversation

sh-ran
Copy link
Collaborator

@sh-ran sh-ran commented Jul 29, 2025

Contributor checklist


Description

The models for the respective entity has been updated to have their own social links model rather than a shared social link model like before. The views have been refactored to handle deletion and updating of old links in a cleaner way. Tests include a 403 error to check if the user who is trying to update the links is either the creator of the group or is a staff/admin.

PS: Thank you @enrouxlibre. Your PR helped a ton while I was working on this issue.

Related issue

Copy link
Contributor

Thank you for the pull request! ❤️

The activist team will do our best to address your contribution as soon as we can. If you're not already a member of our public Matrix community, please consider joining! We'd suggest that you use the Element client as well as Element X for a mobile app, and definitely join the General and Development rooms once you're in. Also consider attending our bi-weekly Saturday developer syncs! It'd be great to meet you 😊

Copy link
Contributor

github-actions bot commented Jul 29, 2025

Maintainer Checklist

The following is a checklist for maintainers to make sure this process goes as well as possible. Feel free to address the points below yourself in further commits if you realize that actions are needed :)

  • The TypeScript, pytest and formatting workflows within the PR checks do not indicate new errors in the files changed

  • The Playwright end to end and Zap penetration tests have been ran and are passing (if necessary)

  • The changelog has been updated with a description of the changes for the upcoming release and the corresponding issue (if necessary)

Copy link

netlify bot commented Jul 29, 2025

Deploy Preview for activist-org canceled.

Name Link
🔨 Latest commit 75d9ecd
🔍 Latest deploy log https://app.netlify.com/projects/activist-org/deploys/688e840595cdea0008a4a7da

@andrewtavis andrewtavis self-requested a review July 29, 2025 15:26
@sh-ran sh-ran requested a review from to-sta July 29, 2025 15:30
@andrewtavis
Copy link
Member

@sh-ran, would you be able to fix the merge conflicts here and then I'll bring this in next? :)

@andrewtavis
Copy link
Member

I need to look into social links now not updating on the frontend, but maybe this is a similar issue with admins not having the right to do changes or make new ones.

@sh-ran
Copy link
Collaborator Author

sh-ran commented Jul 30, 2025

I mean in the tests unless the user has a is_staff attribute the user wont be able to update the links. I look at the code once again. I haven't particularly ran the frontend. Maybe I should have. I will try to fix this ASAP.

@sh-ran
Copy link
Collaborator Author

sh-ran commented Aug 1, 2025

Hey @andrewtavis sorry for getting back to you about this so late. But I think I found why the links wont update in the frontend. So while looking at the logs i saw that while trying to update through the frontend, we are using the group.id. But now that we have added a new standalone model for each links, we now should be using something like groups.socialLinks.id. I also noticed a comment by you within the file frontend/stores/group.ts in the updateSocialLinks function saying that we should be using the id from social links rather than the group id. I tried to fix it but was unable to as I would have to change the interfaces for all the social links and uhhh in simple terms I sucked at it. I can work on this with a bit of guidance from you or Nicole.
Basically we need to update the interfaces for social links to their own entities i.e., group, org and events and pass them in the url.

@andrewtavis
Copy link
Member

Hey @sh-ran 👋 @nicki182 and I are planning on looking in to this tomorrow sometime :) Might be a bit late on your end, but I'll let you know when we decide on a time!

@sh-ran
Copy link
Collaborator Author

sh-ran commented Aug 1, 2025

Sure let me know the timings @andrewtavis and Thank you!

@andrewtavis andrewtavis requested a review from nicki182 August 2, 2025 14:21
@andrewtavis
Copy link
Member

Okkk, @sh-ran 😊 Our idea worked :) I've added create methods into the views and have written the modal such that it saves the original size of the links array, and if there are more links than at the start then the new ones are created :) Is working well. Good enough to bring this in and we can test further in the coming days.

@andrewtavis
Copy link
Member

andrewtavis commented Aug 2, 2025

Reducing the coverage threshold down to 80% for now. We should be able to get it back to 85 and even 90 once #1132 and #1133 are done 😊

Edit: Wasn't necessary in the end :)
Edit edit: Yes it was 😅

@andrewtavis
Copy link
Member

Put methods within the views have been switched to updates to match faqs as the tests were failing without it.

Copy link
Member

@andrewtavis andrewtavis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much for the amazing work here, @sh-ran! Note that on update the order of the social links is at times shifting, but we can figure this out later :) Great to have this level of standardization of the backend now! 🚀

@andrewtavis
Copy link
Member

Tests at 89% locally and 84% here... Will end up reducing the cover threshold.

@andrewtavis andrewtavis merged commit da702d4 into activist-org:main Aug 2, 2025
8 checks passed
@andrewtavis andrewtavis mentioned this pull request Aug 2, 2025
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants