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

Fixing duped report view for admins. Fixes #1933 #1945

Merged
merged 2 commits into from
Nov 23, 2021
Merged

Fixing duped report view for admins. Fixes #1933 #1945

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
821ee81
Fixing duped report view for admins. Fixes #1933
dessalines Nov 23, 2021
e3ee705
Fixing tests.
dessalines Nov 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 35 additions & 24 deletions crates/db_views/src/comment_report_view.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,22 +147,28 @@ impl CommentReportView {
let mut query = comment_report::table
.inner_join(comment::table)
.inner_join(post::table.on(comment::post_id.eq(post::id)))
.inner_join(
community_moderator::table.on(community_moderator::community_id.eq(post::community_id)),
)
.filter(comment_report::resolved.eq(false))
.into_boxed();

// If its not an admin, get only the ones you mod
if !admin {
query = query.filter(community_moderator::person_id.eq(my_person_id));
}

if let Some(community_id) = community_id {
query = query.filter(post::community_id.eq(community_id))
}

query.select(count(comment_report::id)).first::<i64>(conn)
// If its not an admin, get only the ones you mod
if !admin {
query
.inner_join(
community_moderator::table.on(
community_moderator::community_id
.eq(post::community_id)
.and(community_moderator::person_id.eq(my_person_id)),
),
)
.select(count(comment_report::id))
.first::<i64>(conn)
} else {
query.select(count(comment_report::id)).first::<i64>(conn)
}
}
}

Expand Down Expand Up @@ -216,10 +222,6 @@ impl<'a> CommentReportQueryBuilder<'a> {
.inner_join(community::table.on(post::community_id.eq(community::id)))
.inner_join(person::table.on(comment_report::creator_id.eq(person::id)))
.inner_join(person_alias_1::table.on(comment::creator_id.eq(person_alias_1::id)))
// Test this join
.inner_join(
community_moderator::table.on(community_moderator::community_id.eq(post::community_id)),
)
.inner_join(
comment_aggregates::table.on(comment_report::comment_id.eq(comment_aggregates::comment_id)),
)
Expand Down Expand Up @@ -254,11 +256,6 @@ impl<'a> CommentReportQueryBuilder<'a> {
))
.into_boxed();

// If its not an admin, get only the ones you mod
if !self.admin {
query = query.filter(community_moderator::person_id.eq(self.my_person_id));
}

if let Some(community_id) = self.community_id {
query = query.filter(post::community_id.eq(community_id));
}
Expand All @@ -269,11 +266,25 @@ impl<'a> CommentReportQueryBuilder<'a> {

let (limit, offset) = limit_and_offset(self.page, self.limit);

let res = query
.order_by(comment_report::published.asc())
query = query
.order_by(comment_report::published.desc())
.limit(limit)
.offset(offset)
.load::<CommentReportViewTuple>(self.conn)?;
.offset(offset);

// If its not an admin, get only the ones you mod
let res = if !self.admin {
query
.inner_join(
community_moderator::table.on(
community_moderator::community_id
.eq(post::community_id)
.and(community_moderator::person_id.eq(self.my_person_id)),
),
)
.load::<CommentReportViewTuple>(self.conn)?
} else {
query.load::<CommentReportViewTuple>(self.conn)?
};

Ok(CommentReportView::from_tuple_to_vec(res))
}
Expand Down Expand Up @@ -498,8 +509,8 @@ mod tests {
assert_eq!(
reports,
[
expected_sara_report_view.to_owned(),
expected_jessica_report_view.to_owned()
expected_jessica_report_view.to_owned(),
expected_sara_report_view.to_owned()
]
);

Expand Down
59 changes: 35 additions & 24 deletions crates/db_views/src/post_report_view.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,23 +131,28 @@ impl PostReportView {
use diesel::dsl::*;
let mut query = post_report::table
.inner_join(post::table)
// Test this join
.inner_join(
community_moderator::table.on(community_moderator::community_id.eq(post::community_id)),
)
.filter(post_report::resolved.eq(false))
.into_boxed();

// If its not an admin, get only the ones you mod
if !admin {
query = query.filter(community_moderator::person_id.eq(my_person_id));
}

if let Some(community_id) = community_id {
query = query.filter(post::community_id.eq(community_id))
}

query.select(count(post_report::id)).first::<i64>(conn)
// If its not an admin, get only the ones you mod
if !admin {
query
.inner_join(
community_moderator::table.on(
community_moderator::community_id
.eq(post::community_id)
.and(community_moderator::person_id.eq(my_person_id)),
),
)
.select(count(post_report::id))
.first::<i64>(conn)
} else {
query.select(count(post_report::id)).first::<i64>(conn)
}
}
}

Expand Down Expand Up @@ -200,9 +205,6 @@ impl<'a> PostReportQueryBuilder<'a> {
.inner_join(community::table.on(post::community_id.eq(community::id)))
.inner_join(person::table.on(post_report::creator_id.eq(person::id)))
.inner_join(person_alias_1::table.on(post::creator_id.eq(person_alias_1::id)))
.inner_join(
community_moderator::table.on(community_moderator::community_id.eq(post::community_id)),
)
.left_join(
community_person_ban::table.on(
post::community_id
Expand Down Expand Up @@ -234,11 +236,6 @@ impl<'a> PostReportQueryBuilder<'a> {
))
.into_boxed();

// If its not an admin, get only the ones you mod
if !self.admin {
query = query.filter(community_moderator::person_id.eq(self.my_person_id));
}

if let Some(community_id) = self.community_id {
query = query.filter(post::community_id.eq(community_id));
}
Expand All @@ -249,11 +246,25 @@ impl<'a> PostReportQueryBuilder<'a> {

let (limit, offset) = limit_and_offset(self.page, self.limit);

let res = query
.order_by(post_report::published.asc())
query = query
.order_by(post_report::published.desc())
.limit(limit)
.offset(offset)
.load::<PostReportViewTuple>(self.conn)?;
.offset(offset);

// If its not an admin, get only the ones you mod
let res = if !self.admin {
query
.inner_join(
community_moderator::table.on(
community_moderator::community_id
.eq(post::community_id)
.and(community_moderator::person_id.eq(self.my_person_id)),
),
)
Copy link
Member

Choose a reason for hiding this comment

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

You are repeating this code 4 times, maybe you can extract it into a function.

Copy link
Member Author

@dessalines dessalines Nov 23, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Can't unfortunately, the function input query is different for each of these, and it would be 2 pages of BoxedSelectStatement<....tons of table joins>

Copy link
Member

Choose a reason for hiding this comment

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

Damn, i wish they could allow some kind of generics for diesel operations, would save us so much code.

Btw i just noticed that unit tests are failing.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, that pry has to do with my sort order change, I'll fix that in a sec.

.load::<PostReportViewTuple>(self.conn)?
} else {
query.load::<PostReportViewTuple>(self.conn)?
};

Ok(PostReportView::from_tuple_to_vec(res))
}
Expand Down Expand Up @@ -481,8 +492,8 @@ mod tests {
assert_eq!(
reports,
[
expected_sara_report_view.to_owned(),
expected_jessica_report_view.to_owned()
expected_jessica_report_view.to_owned(),
expected_sara_report_view.to_owned()
]
);

Expand Down
2 changes: 2 additions & 0 deletions migrations/2021-11-23-031528_add_report_published_index/down.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
drop index idx_comment_report_published;
drop index idx_post_report_published;
2 changes: 2 additions & 0 deletions migrations/2021-11-23-031528_add_report_published_index/up.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
create index idx_comment_report_published on comment_report (published desc);
create index idx_post_report_published on post_report (published desc);