API: update Search endpoint to be aware of ignored users #8363
Conversation
| FactoryBot.create( | ||
| :block, | ||
| user: auth.user, | ||
| person: eve.person |
There was a problem hiding this comment.
Shouldn't you use someone else than eve here?
There was a problem hiding this comment.
No, we have posts by eve and we block eve. The difference between the first and the second is that the user who is blocking is different (using a different token). This spec structure is a bit misleading, I think but I followed what was already in place
There was a problem hiding this comment.
This looks good to me, but I still have one little comment about the tests. Since it also affects all the other tests and isn't something you did new in this PR, I'm still interested in what you think about it and if you think it's worth changing or if you want to keep it the way it is.
| ) | ||
| expect(response.status).to eq(200) | ||
| posts = response_body_data(response) | ||
| expect(posts.length).to eq(1) |
There was a problem hiding this comment.
I think it would be a good idea to actually check if the one post that gets returned here is the one that we actually expected, so there can't be a bug where there is only one post returned but it's the one from eve?
But on the other hand, all the other specs here also only check the number of posts, but I also think it would be a good idea to check more there too (for example if only public posts should be returned, check if all the posts that got returned are actually public).
|
lgtm! |
|
Merged this, as the change itself is fine 👍 and the tests are all already "bad", so maybe we should just refactor all the test at some point 🤷♂️ Thanks @cmrd-senya for the fix 🍪 |
While I was using insporation I noticed that when I was looking up posts by the tag name it showed up posts by the people I block. It didn't happen in the same case for the web app.
So in this PR I fixed that.
@tclaus looks like I changed the same files as you did in #8284. It means whatever gets shipped first will create conflicts for the other. I'll try to review your PRs, especially the one I referenced.