Skip to content

SharePoint Graph Module - extend functionality with additional methods - #8318

Open
Volodymyr Dvernytskyi (Drakonian) wants to merge 14 commits into
microsoft:mainfrom
Drakonian:GraphSharepointAdditionalFunctionality
Open

SharePoint Graph Module - extend functionality with additional methods#8318
Volodymyr Dvernytskyi (Drakonian) wants to merge 14 commits into
microsoft:mainfrom
Drakonian:GraphSharepointAdditionalFunctionality

Conversation

@Drakonian

@Drakonian Volodymyr Dvernytskyi (Drakonian) commented May 26, 2026 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Contributor

What & why

The SharePoint Graph module currently supports creating files, folders, and list items ΓÇö but offers no way to modify them after creation. Renaming an uploaded file or updating metadata on a list item are among the most common SharePoint integration scenarios, and their absence forces consumers to drop down to raw HTTP calls.

This PR closes that gap by adding update operations for both drive items and list items, backed by the standard MS Graph PATCH endpoints. It also upgrades the test infrastructure to properly cover multi-step operations that were previously untestable.

Linked work

Fixes #8317

How I validated this

  • I read the full diff and it contains only changes I intended.
  • I built the affected app(s) locally with no new analyzer warnings.
  • I ran the change in Business Central and confirmed it behaves as expected.
  • I added or updated tests for the new behavior, or explained below why none are needed.
  • Compiled System Application, Test Library, and Test projects ΓÇö all three build clean, no new analyzer warnings.
  • Ran UpdateDriveItem and RenameDriveItem against a test SharePoint site ΓÇö file renamed successfully, response parsed into SharePoint Graph Drive Item record with updated name.
  • Ran UpdateListItem with a fields payload (Title, custom columns) ΓÇö PATCH succeeded, follow-up GET returned the full list item with updated values.
  • Ran GetListItem for a single item ΓÇö returned the expected record with all fields (ContentType, WebUrl, timestamps).
  • Verified RenameDriveItemByPath and UpdateDriveItemByPath ΓÇö path resolution + PATCH executed as two consecutive HTTP calls, both reflected in mock request log.
  • Confirmed UpdateListItem_FollowUpGetFails test ΓÇö when the GET after PATCH returns 404, the error is surfaced to the caller and request count is 2 (PATCH did fire).
  • Ran all existing SharePoint Graph tests ΓÇö no regressions. Sticky single-response mode works exactly as before.
  • 22 new tests pass: 10 for drive item updates, 8 for list item operations, 4 for multi-response handler infrastructure.

Risk & compatibility

  • All new methods are additive ΓÇö no existing signatures or behavior changed.
  • The test handler refactor preserves the original single-response contract; existing tests work without modification.

Fixes AB#640918

@github-actions github-actions Bot added AL: System Application From Fork Pull request is coming from a fork labels May 26, 2026
@JesperSchulz Jesper Schulz-Wedde (JesperSchulz) added the Integration GitHub request for Integration area label May 27, 2026
@github-actions

github-actions Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

$\textbf{🟡\ Medium\ Severity\ —\ Style} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

No newline at end of file

The file ends without a trailing newline (the diff shows the last added line without the standard Unix EOF newline). While AL compilation typically tolerates this, it causes noise in future diffs and violates standard file conventions in the repository.

Recommendation:

  • Add a trailing newline character at the end of the file after the closing }.
    end;
}

Line mapping was unavailable, so this was posted as an issue comment.

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why

pri-kise
Kilian Seizinger (pri-kise) suggested changes May 27, 2026
View reviewed changes

@pri-kise Kilian Seizinger (pri-kise) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you maybe update?
https://github.com/microsoft/BCApps/blob/main/src/System%20Application/App/SharePoint/README.md

@Groenbech96 Magnus Hartvig Grønbech (Groenbech96) added Approved The issue is approved Linked Issue is linked to a Azure Boards work item labels Jun 30, 2026
@Groenbech96

Magnus Hartvig Grønbech (Groenbech96) commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Build not passing. Merge main in.

@github-actions github-actions Bot added this to the Version 29.0 milestone Jun 30, 2026
@github-actions github-actions Bot added the needs-approval Workflow runs require maintainer approval to start label Jul 4, 2026
@Drakonian
Fix SharePoint Graph mock handler wiring and record population
38b2466 
- Reset the mock HTTP handler in place (Reset/ClearAll) instead of
  Clear(), which detached the instance from an already-initialized client
- Queue full Http Response Message objects in the mock handler so headers
  and reason phrase are preserved; return a diagnostic 500 response
  instead of a hard error when no mock response is queued
- Populate UpdateListItem from the PATCH fieldValueSet response, removing
  the follow-up GET (one round trip, no false failure after a committed
  update)
- Refresh instead of failing on duplicate Insert when the caller's record
  already contains the item (GetListItem, UpdateDriveItem)
- Add ?expand=fields to the single list item endpoint
- Extend tests: buffer reuse, queued header preservation, queue
  exhaustion diagnostics; document mock response modes
@Drakonian
Improve new logic
063b332
@Drakonian
simplify new methods
cdbdf7d
@github-actions

github-actions Bot commented Jul 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Copilot PR Review

Iteration 2 · Outcome: completed

All 16 sub-skills completed against the SharePoint Graph client changes (the diff's true scope; the remaining ~610 changed files in the raw origin/main diff are AL-Go/build infrastructure drift unrelated to this PR's commits and were excluded from review). One sub-skill (al-error-handling-review) surfaced an agent finding; the self-review pass found no additional cross-cutting concerns.

Knowledge source: https://github.com/microsoft/BCQuality@186d8a131465475c79244d994acb872cd5c0d4bf

Findings by domain

Findings split into Knowledge-backed (cite a BCQuality article) and Agent (the agent's own judgement, no matching BCQuality rule).

Domain Findings Knowledge-backed Agent Inline Fallback
Error Handling 1 0 1 0 0

Totals: 0 knowledge-backed · 1 agent findings.

Orchestrator pre-filter (2 file(s) excluded)

  • layer-disabled (knowledge) : 2 file(s)

Findings produced by the Copilot CLI agent against BCQuality at 186d8a131465475c79244d994acb872cd5c0d4bf. Reply 👎 on any inline comment to flag false positives.

@github-actions github-actions Bot removed the needs-approval Workflow runs require maintainer approval to start label Jul 7, 2026
Groenbech96
Magnus Hartvig Grønbech (Groenbech96) previously approved these changes Jul 9, 2026
View reviewed changes
Comment thread src/System Application/App/SharePoint/src/graph/SharePointGraphClientImpl.Codeunit.al Outdated

if ItemId = '' then begin
SharePointGraphResponse.SetError(InvalidItemIdErr);
Session.LogMessage('', InvalidItemIdErr, Verbosity::Error, DataClassification::SystemMetadata, TelemetryScope::ExtensionPublisher, 'Category', GraphSharePointCategoryLbl);

@Groenbech96 Magnus Hartvig Grønbech (Groenbech96) Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aholstrup how do partners generate the telemetry tags?
Lets setup a pipeline that auto pushes that change?

@Groenbech96 Magnus Hartvig Grønbech (Groenbech96) Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alexander Holstrup (@aholstrup1)

@Drakonian
Extend main readme with new methods
8ace8d1 
Add new readme specific for sharepoint graph module
@Drakonian

Volodymyr Dvernytskyi (Drakonian) commented Jul 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Magnus Hartvig Grønbech (@Groenbech96) Jesper Schulz-Wedde (@JesperSchulz)

Should be ready for review/merge, I added new architecture section in main readme of sharepoint module

ofc, after new telemetry calls will have new codes

@github-actions github-actions Bot added needs-approval Workflow runs require maintainer approval to start and removed needs-approval Workflow runs require maintainer approval to start labels Jul 9, 2026
@github-actions github-actions Bot added needs-approval Workflow runs require maintainer approval to start and removed needs-approval Workflow runs require maintainer approval to start labels Jul 15, 2026
@Drakonian

Volodymyr Dvernytskyi (Drakonian) commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

I think someone break my build with wrong number of params for telemetry

@JesperSchulz

Jesper Schulz-Wedde (JesperSchulz) commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

I think someone break my build with wrong number of params for telemetry

Let's try to rerun this! I'll monitor it 😊

github-actions[bot]
github-actions Bot reviewed Aug 12, 2026
View reviewed changes
Comment thread src/System Application/App/SharePoint/src/graph/SharePointGraphClientImpl.Codeunit.al Outdated
github-actions[bot]
github-actions Bot reviewed Aug 12, 2026
View reviewed changes
Comment thread src/System Application/App/SharePoint/src/graph/SharePointGraphClientImpl.Codeunit.al Outdated
@Drakonian

Volodymyr Dvernytskyi (Drakonian) commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

I think someone break my build with wrong number of params for telemetry

Let's try to rerun this! I'll monitor it 😊

Well, I think it's still broken

051f9c0

should I fix it...?

@JesperSchulz

Jesper Schulz-Wedde (JesperSchulz) commented Aug 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

I think someone break my build with wrong number of params for telemetry

Let's try to rerun this! I'll monitor it 😊

Well, I think it's still broken

051f9c0

should I fix it...?

I just quickly looked into the error, and it seems like the system application is broken - unrelated to your PR!? No, you should not look into that! We must fix that right away!

It's strange though. Other builds are passing. Let me investigate!

@JesperSchulz

Jesper Schulz-Wedde (JesperSchulz) commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Volodymyr Dvernytskyi (@Drakonian), seems like all build issues are related to your code after all. Please fix your LogMessage calls to use the right amount of parameters.

@Drakonian

Volodymyr Dvernytskyi (Drakonian) commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Volodymyr Dvernytskyi (Volodymyr Dvernytskyi (@Drakonian)), seems like all build issues are related to your code after all. Please fix your LogMessage calls to use the right amount of parameters.

I would say Magnus code, but I will fix 👌

image

@github-actions github-actions Bot added the needs-approval Workflow runs require maintainer approval to start label Aug 13, 2026
@Drakonian
Add documentation for public test library methods
5b2e574 
Remove customer specific data from telemetry
Make sure that GetListItem couldn't break next API requests
@Drakonian

Volodymyr Dvernytskyi (Drakonian) commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Jesper Schulz-Wedde (@JesperSchulz)

Should be ok now

@github-actions github-actions Bot removed the needs-approval Workflow runs require maintainer approval to start label Aug 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments
@Groenbech96 Magnus Hartvig Grønbech (Groenbech96) Groenbech96 left review comments
+1 more reviewer
@pri-kise Kilian Seizinger (pri-kise) pri-kise requested changes
Reviewers whose approvals may not affect merge requirements

At least 2 approving reviews are required to merge this pull request.

Assignees

@JesperSchulz Jesper Schulz-Wedde (JesperSchulz)

Labels

AL: System Application Approved The issue is approved From Fork Pull request is coming from a fork Integration GitHub request for Integration area Linked Issue is linked to a Azure Boards work item

Projects

None yet

Milestone

Version 29.0

Development

Successfully merging this pull request may close these issues.

[BC Idea]: SharePoint Graph Module - extend functionality with additional methods

4 participants

@Drakonian @Groenbech96 @JesperSchulz @pri-kise