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.

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

GetComments() paging support doesn't work when selectors is passed #1424

Closed
1 task done
larry-lau opened this issue Mar 14, 2024 · 1 comment
Closed
1 task done
Assignees
Labels
area: model 📐 Related to the core SDK models area: pages API 📄 Working with modern pages

Comments

@larry-lau
Copy link

Category

  • Bug

Describe the bug

The new paging support introduced in 1.12 works when you call GetComments() without selectors parameters.
It doesn't work when selectors is passed. i.e. It returns only 30 comments even when there are more on the page.

This is related to #1361

Steps to reproduce

See code block below
Add the following unit test to the PagesTests.cs in PnP.Core.Test project
Run the test without mocking turned off.
Assertion failure since it returns 30 comments instead of 45 that get added in this test.

[TestMethod]
public async Task PageCommentingTestOnlyReturn30CommentsWithSelector()
{
    TestCommon.Instance.Mocking = false;
    using (var context = await TestCommon.Instance.GetContextAsync(TestCommon.TestSite))
    {
        IPage newPage = null;
        try
        {
            newPage = await context.Web.NewPageAsync();
            string pageName = TestCommon.GetPnPSdkTestAssetName("PageCommentingTestOnlyReturn30Comments.aspx");

            // Save the page
            await newPage.SaveAsync(pageName);

            // Publish the page, required before it can be liked
            newPage.Publish();

            // Get Page comments                
            var comments = newPage.GetComments();
            
            Assert.IsTrue(comments.Length == 0);

            var noCommentsAdded = 45;

            foreach (var i in Enumerable.Range(1, noCommentsAdded))
            {
                // Add a comment
                await comments.AddBatchAsync($"Comment #: {i} added by unit test");
            }

            await context.ExecuteAsync();

            comments = newPage.GetComments(
                p => p.Author,
                p => p.Text,
                p => p.ReplyCount,
                p => p.CreatedDate,
                p => p.Replies);

            // Expecting 45 but only 30 is returned. 
            Assert.IsTrue(comments.Length == noCommentsAdded);

        }
        finally
        {
            // Delete the page
            await newPage.DeleteAsync();
        }
    }
}

Expected behavior

Calling GetComments() with selectors parameter should return all comments.
var comments = page.GetComments(
p => p.Author,
p => p.Text,
p => p.ReplyCount,
p => p.CreatedDate,
p => p.Replies);

Environment details (development & target environment)

  • SDK version: [1.12.0]
  • OS: [Windows 10]
  • SDK used in: [Console App]
  • Framework: [.NET 8 ]
  • Browser(s): [NA]
  • Tooling: [Visual Studio 2022]
  • Additional details: The more context you can provide, the easier it is (and therefore quicker) to help.

Additional context

After reviewing the code I noticed that in the code branch that handle when selectors is provided, the constructor call to ApiCall should have loadPages: true parameter. Specifically the BuildGetCommentsApiCallAsync method in ListItem.cs
Line 1703: return new ApiCall(query.ApiCall.Request, ApiType.SPORest, receivingProperty: nameof(Comments), loadPages: true);

Thanks for your contribution! Sharing is caring.

@jansenbe jansenbe self-assigned this Mar 14, 2024
@jansenbe jansenbe added area: model 📐 Related to the core SDK models area: pages API 📄 Working with modern pages labels Mar 14, 2024
@jansenbe
Copy link
Contributor

@larry-lau : thanks for the detailed write up and analysis, I've fixed the code like you mentioned and updated the test case to cover this scenario. Fix will be part of the next nightly, closing issue now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: model 📐 Related to the core SDK models area: pages API 📄 Working with modern pages
Projects
None yet
Development

No branches or pull requests

2 participants