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
Get posts for a channel has_next attribute fix #26901
base: master
Are you sure you want to change the base?
Conversation
Hello @Aryakoste, Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here. |
E2E tests not automatically triggered, because the PR is not in a mergeable state. Please update the branch with the base branch and resolve outstanding conflicts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, as I mentioned here: #26862 (comment), please add a test in the getPostsForChannel API endpoint (see api4/post_test.go) and confirm that the pointer is nil in the response.
@@ -776,7 +776,7 @@ func testPostStoreGetForThread(t *testing.T, rctx request.CTX, ss store.Store) { | |||
require.NoError(t, err) | |||
require.Len(t, r1.Order, 3) // including the root post | |||
require.Len(t, r1.Posts, 3) | |||
assert.True(t, r1.HasNext) | |||
require.Nil(t, r1.HasNext) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes seem wrong. The assertion should still be true here. I would assume something like: assert.True(t, *r1.HasNext)
after a require.NotNil(t, r1.HasNext)
@Aryakoste - please check the CI for failures
The code needs to be changed as well for the tests to work. |
Summary
Fixed the issue where has_next attribute always returned false. Made has_next attribute pointer and made changes accordingly.
Ticket Link
Fixes #26862
Jira https://mattermost.atlassian.net/browse/MM-57937
Screenshots
Release Note