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.

Sign up for GitHub

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

Jump to bottom

Add moderator view parameter to list posts #3176

Merged
merged 21 commits into from
Jul 26, 2023
Merged

Add moderator view parameter to list posts #3176

merged 21 commits into from
Jul 26, 2023

Conversation

biosfood
Copy link
Contributor

@biosfood biosfood commented Jun 18, 2023 •
edited
Loading

Add a new parameter to list posts: moderated_only to allow for a "moderator view"

@biosfood
Copy link
Contributor Author

Note: woodpecker seems not to run when force-pushing the PR to correct the formatting?

@biosfood biosfood mentioned this pull request Jun 18, 2023
Closed
@biosfood biosfood marked this pull request as ready for review June 20, 2023 12:52
@Nutomic
Copy link
Member

Nutomic commented Jun 20, 2023

Can you explain why this would be useful?

@biosfood
Copy link
Contributor Author

This was first brought up in LemmyNet/lemmy-ui#1294 and would allow a moderator of multiple communities to open the app and filter only the posts they need to moderate without being cluttered by other stuff.

@poVoq
Copy link

poVoq commented Jun 22, 2023

Maybe if this would the same time show posts from individually blocked users?

Right now moderators can't really block any users individually as they will not be able to see the posts of these people then.

But a "moderation view mode" that shows only moderated communities and shows all users regardless of block status might be a nice feature.

@biosfood
Copy link
Contributor Author

But a "moderation view mode" that shows only moderated communities and shows all users regardless of block status might be a nice feature.

That seems to be a rather important so I will probably rename moderated_only to moderator_view and disable blocking users while that is active.

@biosfood biosfood changed the title add option to only show posts from moderated communities Add moderator view parameter to list posts Jun 24, 2023
dessalines
dessalines reviewed Jun 27, 2023
View reviewed changes
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.

Add a unit test for this one too if you would, seems pretty important.

crates/api_common/src/post.rs Show resolved Hide resolved
dessalines
dessalines reviewed Jul 3, 2023
View reviewed changes
Copy link
Member

@dessalines dessalines left a comment •
edited
Loading

Choose a reason for hiding this comment

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

You need to run cargo +nightly fmt , and resolve the conflicts. After that, I can do a lemmy-js-client release with this included, and give you the version #.

api_tests/src/shared.ts Outdated Show resolved Hide resolved
crates/api_common/src/post.rs Show resolved Hide resolved
@biosfood biosfood closed this Jul 8, 2023
@biosfood biosfood reopened this Jul 8, 2023
@biosfood biosfood requested a review from phiresky as a code owner July 11, 2023 19:51
dessalines
dessalines approved these changes Jul 12, 2023
View reviewed changes
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.

This looks good now. @Nutomic once you approve I'll start a lemmy-js-client release with this included.

@dessalines dessalines requested a review from Nutomic July 12, 2023 12:15
dessalines
dessalines requested changes Jul 12, 2023
View reviewed changes
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.

Okay use this new lemmy-js-client: 0.18.3-rc.1

@biosfood biosfood requested a review from dessalines July 12, 2023 14:36
phiresky
phiresky approved these changes Jul 13, 2023
View reviewed changes
Copy link
Collaborator

@phiresky phiresky left a comment

Choose a reason for hiding this comment

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

Code looks ok

@Nutomic Nutomic enabled auto-merge (squash) July 14, 2023 13:19
@Nutomic Nutomic mentioned this pull request Jul 14, 2023
Merged
auto-merge was automatically disabled July 18, 2023 09:02

Head branch was pushed to by a user without write access

dessalines
dessalines reviewed Jul 19, 2023
View reviewed changes
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.

Federation tests are failing for some reason, but if you get those working, we can merge.

api_tests/src/shared.ts Outdated Show resolved Hide resolved
@biosfood
Copy link
Contributor Author

biosfood commented Jul 19, 2023 •
edited
Loading

I figured out why the federation tests keep failing, lemmy-js-client throws exceptions if a request fails (for example a community already exists -> error).

There has been a new major version in the lemmy-js-client and a bunch of subversions so I don't quite know what the culprit is (probably https://github.com/LemmyNet/lemmy-js-client/blob/main/src/http.ts#L1333C1-L1345C6 here) and the older versions don't cause those exceptions.

Adding errorOnUnhandledRejections: false in the jest config seems to fix some issues but would obviously not be a good idea.

How should I try and fix the remaining problems?

@dessalines
Copy link
Member

How should I try and fix the remaining problems?

There are examples in those jest tests of catching and ignoring errors.

Also conflicts need fixed.

@dessalines
Copy link
Member

I have the federation tests fixed in a PR, once that gets merged, you should be okay.

@biosfood
Copy link
Contributor Author

Everything should work fine now, it would be great if you could squash all the commits for merging them, as some of the history here is a bit messed up now

@biosfood biosfood requested a review from dessalines July 23, 2023 19:14
dessalines
dessalines requested changes Jul 25, 2023
View reviewed changes
crates/db_views/src/post_view.rs Outdated Show resolved Hide resolved
crates/db_views/src/post_view.rs Outdated Show resolved Hide resolved
crates/db_views/src/post_view.rs Outdated Show resolved Hide resolved
@biosfood biosfood requested a review from dessalines July 25, 2023 21:02
dessalines
dessalines approved these changes Jul 26, 2023
View reviewed changes
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.

Much better, thanks.

@dessalines dessalines enabled auto-merge (squash) July 26, 2023 17:13
@dessalines dessalines merged commit 2de9947 into LemmyNet:main Jul 26, 2023
This was referenced Jul 27, 2023
Closed
Closed
Merged
@Nutomic Nutomic mentioned this pull request Oct 2, 2023
Closed
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@phiresky phiresky phiresky approved these changes

@Nutomic Nutomic Nutomic approved these changes

@dessalines dessalines dessalines approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@biosfood @Nutomic @poVoq @dessalines @phiresky