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

Follow-up on "Can't update list item #399" #508

Closed
wonderplayer opened this issue Jul 7, 2021 · 7 comments
Closed

Follow-up on "Can't update list item #399" #508

wonderplayer opened this issue Jul 7, 2021 · 7 comments
Assignees
Labels
area: model 📐 Related to the core SDK models

Comments

@wonderplayer
Copy link
Contributor

This is a follow-up issue to #399 issue, as unfortunately it wasn't resolved with that resolution of #485 issue.
I am still getting exception, when trying to update a list item after getting it by ID.

@jansenbe
Copy link
Contributor

jansenbe commented Jul 7, 2021

@wonderplayer : which version did you test with? I did another related fix yesterday, so if you've tested with a version older than today's then please give the latest version a quick try. Note that I'm OOF, so will take some time to dive into the details

@jansenbe jansenbe self-assigned this Jul 7, 2021
@jansenbe jansenbe added the area: model 📐 Related to the core SDK models label Jul 7, 2021
@wonderplayer
Copy link
Contributor Author

I tested with 1.2.30-nightly

@jansenbe
Copy link
Contributor

@wonderplayer : I'm not able to reproduce the issue. I've built below test case on this one correctly updates the item when it was loaded using GetByIdAsync. Both CSOM driven updates (SystemUpdate/UpdateOverwriteVersion) as REST updates (Update) work fine in my test.

Does below test mimic your setup? If not can you help me build a repro for this issue?

[TestMethod]
public async Task PagesListItemUpdate()
{
    TestCommon.Instance.Mocking = false;
    using (var context = await TestCommon.Instance.GetContextAsync(TestCommon.TestSite))
    {
        // Create a new sub site to test as we don't want to break the main site site pages library
        string webTitle = "PagesListItemUpdate";
        var addedWeb = await context.Web.Webs.AddAsync(new WebOptions { Title = webTitle, Url = webTitle });

        // Create a context for the newly created web
        using (var context2 = await TestCommon.Instance.CloneAsync(context, addedWeb.Url, 1))
        {
            // Add a new field to the pages library
            var list = await context2.Web.Lists.GetByTitleAsync("Site Pages", p => p.Fields);
            var field = await list.Fields.AddTextAsync("CustomTextField");

            // Read the current home page, ID = 1
            var homePage = await list.Items.GetByIdAsync(1);

            // Set the metadata
            homePage.Values["CustomTextField"] = "yeah";
            
            // Both REST as CSOM based approaches work
            //await homePage.UpdateAsync();
            await homePage.SystemUpdateAsync();

            // Read the home page again
            list.Items.Clear();
            homePage = await list.Items.GetByIdAsync(1);

            Assert.AreEqual(homePage.Values["CustomTextField"].ToString(), "yeah");
        }

        // Delete the web to cleanup the test artefacts
        await addedWeb.DeleteAsync();
    }
}

@wonderplayer
Copy link
Contributor Author

@jansenbe
So, your setup really does work for me. But here is the trick - only your example does.
If I update any other page except home page, I get an exception. Pretty funny 😄

@jansenbe
Copy link
Contributor

@wonderplayer : thanks for the feedback, seems to be tricky issue :-) I'm back from vacation, so will have a look at this one again

@jansenbe
Copy link
Contributor

@wonderplayer : tried with adding a new page and then setting the page metadata and that did work as well (see below code), but adding the page manually and then trying to set the metadata failed...so we have a repro! Will update this thread once I know more

[TestMethod]
public async Task PagesListItemUpdate()
{
    TestCommon.Instance.Mocking = false;
    using (var context = await TestCommon.Instance.GetContextAsync(TestCommon.TestSite))
    {
        // Create a new sub site to test as we don't want to break the main site site pages library
        string webTitle = "PagesListItemUpdate";
        var addedWeb = await context.Web.Webs.AddAsync(new WebOptions { Title = webTitle, Url = webTitle });

        // Create a context for the newly created web
        using (var context2 = await TestCommon.Instance.CloneAsync(context, addedWeb.Url, 1))
        {
            // Add a new field to the pages library
            var list = await context2.Web.Lists.GetByTitleAsync("Site Pages", p => p.Fields);
            var field = await list.Fields.AddTextAsync("CustomTextField");

            // Add a custom page
            var newPage = await context2.Web.NewPageAsync();
            string pageName = TestCommon.GetPnPSdkTestAssetName("PagesListItemUpdate.aspx");
            // Save the page
            await newPage.SaveAsync(pageName);

            // Read the newly added page, ID = 2
            var addedPage = await list.Items.GetByIdAsync(2);

            // Set the metadata
            addedPage.Values["CustomTextField"] = "yeah";

            // Both REST as CSOM based approaches work
            //await homePage.UpdateAsync();
            await addedPage.SystemUpdateAsync();

            // Read the home page again
            list.Items.Clear();
            addedPage = await list.Items.GetByIdAsync(2);

            Assert.AreEqual(addedPage.Values["CustomTextField"].ToString(), "yeah");
        }

        // Delete the web to cleanup the test artefacts
        await addedWeb.DeleteAsync();
    }
}

jansenbe added a commit that referenced this issue Aug 24, 2021
@jansenbe
Copy link
Contributor

@wonderplayer : when loading list items using the "regular" approach some fields (like _AuthorByLine) were wrongly marked as changed after the load. When using the ListDataAsStream approach we did clear all fields marked as changed after the load, we're now doing the same for the "regular" approach as well.

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
Projects
None yet
Development

No branches or pull requests

2 participants