Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add banned_from_community to PostView and CommentView #4552

Merged
merged 4 commits into from Mar 22, 2024

Conversation

SleeplessOne1917
Copy link
Member

Similar to #4458.

I went to try to implement this for the frontend after #4458 and found that even with banned_from_community on the CommunityView, trying to use this information at best requires a lot of prop drilling and at worst (when viewing posts/comments outside of the context of a community) requires the convoluted approaches I listed in #4458.

dullbananas
dullbananas previously approved these changes Mar 19, 2024
@Nutomic
Copy link
Member

Nutomic commented Mar 19, 2024

Please add some test coverage, either via unit tests or api_tests. Its probably enough to add some extra asserts in existing tests rather than writing a new one.

@@ -77,7 +77,7 @@ impl CustomEmojiView {
}
for emoji in &mut result {
if let Some(keywords) = hash.get_mut(&emoji.custom_emoji.id) {
emoji.keywords = keywords.clone();
emoji.keywords.clone_from(keywords);
Copy link
Member Author

Choose a reason for hiding this comment

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

scripts/lint.sh did this.

Copy link
Member

@dessalines dessalines left a comment

Choose a reason for hiding this comment

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

Looks good, thx.

@dessalines dessalines merged commit 38c22d9 into main Mar 22, 2024
2 checks passed
@SleeplessOne1917 SleeplessOne1917 deleted the more-banned-from-comm branch March 22, 2024 22:52
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.

None yet

4 participants