[Feature] add setting site logo support for group with applications perms by danielpastoor · Pull Request #1519 · pnp/pnpcore

danielpastoor · 2024-09-01T15:40:54Z

I changed the api call from SharePoint to the Graph api call so it is now possible to set the site logo of a group with application permission and still support it for delegated permissions.

Also I had to change the ApiCall.cs because you need to add the content-type to the ByteArrayContent for uploading the image to the Graph Api.

…the ApiCall class so it is possible to add a content-type to the ByteArrayContent. This is needed for uploading images or other files to the graph api

…nail when the site is connected to a group

… logo is not supported for group connected sites with application permission

danielpastoor · 2024-09-01T15:52:51Z

@jansenbe I saw that the "Build and Test" failed but I can't find why this is related to the code that I changed. Do you maybe know if this is related?

Adam-it

@danielpastoor solid work 👏 and sorry for the huge delay on our side.
May I kindly ask you to go over the few review comments I added before we merge

Adam-it · 2026-05-08T21:22:47Z

-                    Interactive = true,
-                    BinaryBody = ToByteArray(content),
-                    Headers = headers
+                    Interactive = true, Content = byteContent


Suggested change

Interactive = true, Content = byteContent

Interactive = true,

Content = byteContent

Adam-it · 2026-05-08T21:26:31Z

                if (binaryContent != null)
                {
                    binaryContent.Dispose();
                }


I suppose this is not needed now

Adam-it · 2026-05-08T21:26:57Z

        {
            var graphRequest = batch.Requests.First().Value;
            StringContent content = null;
            ByteArrayContent binaryContent = null;


I guess we do not use this variable now so we could remove it

Adam-it · 2026-05-08T21:33:49Z

+                    PnPContext.Web.SiteLogoUrl = correctSiteLogoUrl;
+                    await PnPContext.Web.UpdateAsync().ConfigureAwait(false);
+
+                    const string cachedSiteIcon = "__siteIcon__.png";


Seems a bit strange in Reset method we use __siteIcon__.jpg. Maybe we should align

Adam-it · 2026-05-08T21:42:57Z

+
+                // Use StartSWith to avoid issues with the query string that can contains &hash=xxxx
+                if (string.IsNullOrEmpty(PnPContext.Web.SiteLogoUrl) ||
+                    !PnPContext.Web.SiteLogoUrl.StartsWith(correctSiteLogoUrl))


won't this always resolve to false as the SiteLogoUrl is a full absolute URL, so something like

https://contoso.sharepoint.com/sites/mysite/_api/GroupService/GetGroupImage?id='xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx'&hash=12345

But the correctSiteLogoUrl, since it uses PathAndQuery will end up as something like:

/sites/mysite/_api/GroupService/GetGroupImage?id='xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx'

Right?

Copilot

Pull request overview

This PR updates how binary payloads (e.g., images) are sent so the SDK can set the logo for Microsoft 365 group–connected sites using application permissions, by switching the group-logo update to a Microsoft Graph call and by carrying binary request content via HttpContent.

Changes:

Replace ApiCall.BinaryBody (byte[]) with ApiCall.Content (HttpContent) and flow it through interactive request execution.
Update site/group logo thumbnail upload for group-connected sites to use PUT groups/{groupId}/photo/$value (Graph), including setting the content type.
Remove documentation warning stating application permissions are unsupported for these logo methods.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/sdk/PnP.Core/Services/Core/BatchClient.cs	Uses `ApiCall.Content` for interactive Graph/SPO REST requests instead of constructing `ByteArrayContent` from `BinaryBody`.
src/sdk/PnP.Core/Services/Core/Batch.cs	Copies `ApiCall.Content` when preparing batched requests.
src/sdk/PnP.Core/Services/Core/ApiCall.cs	Replaces `BinaryBody` with `HttpContent Content` in the core request descriptor.
src/sdk/PnP.Core/Model/SharePoint/Core/Internal/UserProfile.cs	Uses `ByteArrayContent` for profile picture upload via `ApiCall.Content`.
src/sdk/PnP.Core/Model/SharePoint/Core/Internal/FileCollection.cs	Uses `ByteArrayContent` for file upload via `ApiCall.Content`.
src/sdk/PnP.Core/Model/SharePoint/Core/Internal/FieldThumbnailValue.cs	Uses `ByteArrayContent` for image upload via `ApiCall.Content`.
src/sdk/PnP.Core/Model/SharePoint/Core/Internal/AttachmentCollection.cs	Uses `ByteArrayContent` for attachment upload via `ApiCall.Content`.
src/sdk/PnP.Core/Model/SharePoint/Branding/Internal/HeaderOptions.cs	Switches group-connected site logo upload to Graph and adds post-update SiteLogoUrl/cache handling.
src/sdk/PnP.Core.Admin/Model/SharePoint/Core/Internal/AppManager.cs	Uses `ByteArrayContent` for app upload via `ApiCall.Content`.
docs/using-the-sdk/branding-intro.md	Removes warning about application-permission limitation for group-connected site logo APIs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+                    else if (graphRequest.ApiCall.Content != null)
                    {
-                        binaryContent = new ByteArrayContent(graphRequest.ApiCall.BinaryBody);
-                        request.Content = binaryContent;
+                        request.Content = graphRequest.ApiCall.Content;
                    }


+                    else if (restRequest.ApiCall.Content != null)
                    {
-                        binaryContent = new ByteArrayContent(restRequest.ApiCall.BinaryBody);
-                        request.Content = binaryContent;
+                        request.Content = restRequest.ApiCall.Content;
                    }


+                // Use StartSWith to avoid issues with the query string that can contains &hash=xxxx
+                if (string.IsNullOrEmpty(PnPContext.Web.SiteLogoUrl) ||
+                    !PnPContext.Web.SiteLogoUrl.StartsWith(correctSiteLogoUrl))


+
+                    try
+                    {
+                        // check if we have a file named "__siteIcon__.jpg" in the SiteAssets folder


+                var apiCall = new ApiCall($"groups/{PnPContext.Site.GroupId}/photo/$value", ApiType.Graph)
                {
-                    Interactive = true,
-                    BinaryBody = ToByteArray(content),
-                    Headers = headers
+                    Interactive = true, Content = byteContent
                };


Adam-it · 2026-06-03T19:40:26Z

@danielpastoor is there any chance you are still interested in refreshing this PR and resolving the comments I gave?
Totally understandable if not since your PR was opened very long time ago and we did not have any chance to commit to it so it's 100% on us, but if you are ready for another round I'm here to help and process the PR 🙂

danielpastoor added 3 commits September 1, 2024 17:26

Changed BinaryBody property to Content so the type is HttpContent in …

b995b35

…the ApiCall class so it is possible to add a content-type to the ByteArrayContent. This is needed for uploading images or other files to the graph api

Added support for application permissions to set a site logo or thumb…

d0df2f5

…nail when the site is connected to a group

Removed the part in the documentation where it said that setting site…

4c53ede

… logo is not supported for group connected sites with application permission

jansenbe self-assigned this Sep 9, 2024

jansenbe added the area: model 📐 Related to the core SDK models label Sep 9, 2024

Adam-it self-assigned this May 8, 2026

Adam-it requested changes May 8, 2026

View reviewed changes

Adam-it marked this pull request as draft May 8, 2026 21:44

Adam-it requested a review from Copilot May 8, 2026 21:44

Copilot started reviewing on behalf of Adam-it May 8, 2026 21:45 View session

Copilot AI reviewed May 8, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature] add setting site logo support for group with applications perms#1519

[Feature] add setting site logo support for group with applications perms#1519
danielpastoor wants to merge 3 commits into
pnp:devfrom
danielpastoor:feature/add-site-logo-support-for-group-with-applications-perms

danielpastoor commented Sep 1, 2024

Uh oh!

danielpastoor commented Sep 1, 2024

Uh oh!

Adam-it left a comment

Uh oh!

Adam-it May 8, 2026

Uh oh!

Adam-it May 8, 2026

Uh oh!

Adam-it May 8, 2026

Uh oh!

Adam-it May 8, 2026

Uh oh!

Adam-it May 8, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Adam-it commented Jun 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	Interactive = true, Content = byteContent
	Interactive = true,
	Content = byteContent

Conversation

danielpastoor commented Sep 1, 2024

Uh oh!

danielpastoor commented Sep 1, 2024

Uh oh!

Adam-it left a comment

Choose a reason for hiding this comment

Uh oh!

Adam-it May 8, 2026

Choose a reason for hiding this comment

Uh oh!

Adam-it May 8, 2026

Choose a reason for hiding this comment

Uh oh!

Adam-it May 8, 2026

Choose a reason for hiding this comment

Uh oh!

Adam-it May 8, 2026

Choose a reason for hiding this comment

Uh oh!

Adam-it May 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Adam-it commented Jun 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants