Skip to content

Commit

Permalink
FIX: Hide live-loaded posts from ignored users
Browse files Browse the repository at this point in the history
  • Loading branch information
davidtaylorhq committed Jul 25, 2019
1 parent 0c7df55 commit 4f1382a
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 11 deletions.
9 changes: 7 additions & 2 deletions app/assets/javascripts/discourse/models/post-stream.js.es6
Original file line number Diff line number Diff line change
Expand Up @@ -609,9 +609,14 @@ export default RestModel.extend({
this.set("loadingLastPost", true);
return this.findPostsByIds([postId])
.then(posts => {
const ignoredUsers = this.get("currentUser.ignored_users");
const ignoredUsers =
Discourse.User.current() &&

This comment has been minimized.

Copy link
@eviltrout

eviltrout Jul 25, 2019

Contributor

Can you explain why you did this instead of this.currentUser? They should be the same, and in the long term we want to remove Discourse.XYZ constants.

This comment has been minimized.

Copy link
@davidtaylorhq

davidtaylorhq Jul 25, 2019

Author Member

currentUser doesn't get injected into models. It only gets injected into components, routes and controllers.

https://github.com/discourse/discourse/blob/master/app/assets/javascripts/discourse/pre-initializers/inject-discourse-objects.js.es6#L83-L85

This comment has been minimized.

Copy link
@eviltrout

eviltrout Jul 25, 2019

Contributor

In that case we should be passing the current user into the finder method instead

Discourse.User.current().get("ignored_users");
posts.forEach(p => {
if (ignoredUsers && ignoredUsers.includes(p.username)) return;
if (ignoredUsers && ignoredUsers.includes(p.username)) {
this.stream.removeObject(postId);
return;
}
this.appendPost(p);
});
})
Expand Down
38 changes: 29 additions & 9 deletions test/javascripts/models/post-stream-test.js.es6
Original file line number Diff line number Diff line change
Expand Up @@ -803,12 +803,14 @@ QUnit.test("comitting and triggerNewPostInStream race condition", assert => {
QUnit.test("triggerNewPostInStream for ignored posts", async assert => {
const postStream = buildStream(280, [1]);
const store = postStream.store;
postStream.currentUser = Discourse.User.create({
username: "eviltrout",
name: "eviltrout",
id: 321,
ignored_users: ["ignoreduser"]
});
Discourse.User.resetCurrent(
Discourse.User.create({
username: "eviltrout",
name: "eviltrout",
id: 321,
ignored_users: ["ignoreduser"]
})
);

postStream.appendPost(store.createRecord("post", { id: 1, post_number: 1 }));

Expand All @@ -829,13 +831,31 @@ QUnit.test("triggerNewPostInStream for ignored posts", async assert => {
.returns(Promise.resolve([post2]));

await postStream.triggerNewPostInStream(101);
assert.equal(postStream.get("posts.length"), 2, "it added the regular post");
assert.equal(
postStream.posts.length,
2,
"it added the regular post to the posts"
);
assert.equal(
postStream.get("stream.length"),
2,
"it added the regular post to the stream"
);

stub.restore();
sandbox.stub(postStream, "findPostsByIds").returns(Promise.resolve([post3]));

postStream.triggerNewPostInStream(102);
assert.equal(postStream.posts.length, 2, "it does not add the ignored post");
await postStream.triggerNewPostInStream(102);
assert.equal(
postStream.posts.length,
2,
"it does not add the ignored post to the posts"
);
assert.equal(
postStream.stream.length,
2,
"it does not add the ignored post to the stream"
);
});

QUnit.test("postsWithPlaceholders", assert => {
Expand Down

2 comments on commit 4f1382a

@discoursebot
Copy link

Choose a reason for hiding this comment

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

This commit has been mentioned on Discourse Meta. There might be relevant details there:

https://meta.discourse.org/t/ability-to-ignore-a-user/110254/58

@discoursereviewbot
Copy link

Choose a reason for hiding this comment

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

David Taylor posted:

This was resolved in f9894ae

Please sign in to comment.