Skip to content

Adds spp autofillcolumn apply command. Closes #6203#6460

Merged
Adam-it merged 1 commit intopnp:mainfrom
mkm17:issues/6203_spp_autofillcolumn-apply
Jan 26, 2026
Merged

Adds spp autofillcolumn apply command. Closes #6203#6460
Adam-it merged 1 commit intopnp:mainfrom
mkm17:issues/6203_spp_autofillcolumn-apply

Conversation

@mkm17
Copy link
Copy Markdown
Contributor

@mkm17 mkm17 commented Nov 1, 2024

Adds spp autofillcolumn apply command. Closes #6203

Hi, just a few remarks on this command:

I’ve added the columnId and columnTitle parameters to allow selection of the column by either option.

I noticed that retrieving the list instance, like in the getDocumentLibraryInfo function, appears in several places in the solution. Could you advise on where we should store this common code? Is spo.ts the correct file for it?

Similarly, for retrieving a field, should the getColumn function also be placed in spo.ts?

Do you think I should make these updates in this PR, or would it be better to create a separate one?

@mkm17 mkm17 force-pushed the issues/6203_spp_autofillcolumn-apply branch from 2ff3284 to 50c5529 Compare November 1, 2024 21:22
@milanholemans
Copy link
Copy Markdown
Contributor

Thank you, we'll try to review it ASAP!

@milanholemans
Copy link
Copy Markdown
Contributor

Sorry for the late reply @mkm17, I totally missed your remark last time.

I noticed that retrieving the list instance, like in the getDocumentLibraryInfo function, appears in several places in the solution. Could you advise on where we should store this common code? Is spo.ts the correct file for it?

Is this something we are using at a lot of other spots? Seems like in your code you are only retrieving the list BaseType. I don't know if we need this in other places?

Similarly, for retrieving a field, should the getColumn function also be placed in spo.ts?

SPO util will be the right place I guess, yes.

Do you think I should make these updates in this PR, or would it be better to create a separate one?

To me, it kind of depends on how many places we'll have to replace code by this util function. If it's just one other command, you can easily add it to this PR. If there are like 5 other commands sharing the same logic, I'd opt for a separate issue/PR.

@mkm17 mkm17 marked this pull request as draft January 26, 2025 19:17
@mkm17 mkm17 force-pushed the issues/6203_spp_autofillcolumn-apply branch 2 times, most recently from 064b2f6 to db18aae Compare January 29, 2025 21:17
@mkm17
Copy link
Copy Markdown
Contributor Author

mkm17 commented Jan 29, 2025

Hi, according to the latest comments from @martinlingstuyl on the specification, I have changed the current apply command to set.

The prompt and isEnabled parameters are optional if the selected column already has the autofill setting applied. In the update case, we can modify both of these settings.

I have used the column property AutofillInfo to check if it is an update and the _api/machinelearning/SetColumnLLMInfo endpoint to update the autofill column settings.

I noticed an issue with the test stage in the PR checks, but all tests pass when I run them locally. Can we rerun these checks without changing the code?

@milanholemans
Copy link
Copy Markdown
Contributor

I noticed an issue with the test stage in the PR checks, but all tests pass when I run them locally. Can we rerun these checks without changing the code?

I just did rerun them, but it seems like it doesn't help. I think something is wrong with the macOs pipeline, in the last couple of days I saw this pipeline failing on other PRs as well. We'll have to take a look at it.

@milanholemans milanholemans force-pushed the issues/6203_spp_autofillcolumn-apply branch from db18aae to d476d53 Compare February 3, 2025 21:46
@milanholemans
Copy link
Copy Markdown
Contributor

Hi @mkm17, I rebased your PR to the latest main to see if our pipeline has been fixed. I hope you don't mind if we take you as a test. Since I performed a force push, I recommend you delete your local branch and pull again from your forked repo.

@mkm17 mkm17 force-pushed the issues/6203_spp_autofillcolumn-apply branch 2 times, most recently from 779edbc to 81f60aa Compare July 9, 2025 21:04
@mkm17 mkm17 marked this pull request as ready for review July 9, 2025 21:08
@mkm17
Copy link
Copy Markdown
Contributor Author

mkm17 commented Jul 9, 2025

Hi @milanholemans sorry for the delay. I've finally made some changes to this command. it's now ready for review.

@martinlingstuyl martinlingstuyl self-assigned this Oct 8, 2025
Copy link
Copy Markdown
Contributor

@martinlingstuyl martinlingstuyl left a comment

Choose a reason for hiding this comment

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

Hi @mkm17, I've reviewed your PR, looks like solid work. I do have some comments. Could you look into those and fix them?

Comment thread docs/docs/cmd/spp/autofillcolumn/autofillcolumn-set.mdx Outdated
Comment thread src/m365/spp/commands/autofillcolumn/autofillcolumn-set.ts Outdated
Comment thread src/m365/spp/commands/autofillcolumn/autofillcolumn-set.ts
Comment thread src/m365/spp/commands/autofillcolumn/autofillcolumn-set.ts Outdated
Comment thread src/m365/spp/commands/autofillcolumn/autofillcolumn-set.ts Outdated
Comment thread src/m365/spp/commands/autofillcolumn/autofillcolumn-set.ts
Comment thread src/m365/spp/commands/autofillcolumn/autofillcolumn-set.ts
Comment thread src/m365/spp/commands/autofillcolumn/autofillcolumn-set.ts
Comment thread src/m365/spp/commands/autofillcolumn/autofillcolumn-set.ts Outdated
Comment thread src/m365/spp/commands/autofillcolumn/autofillcolumn-set.spec.ts Outdated
@martinlingstuyl martinlingstuyl marked this pull request as draft November 3, 2025 15:46
@martinlingstuyl
Copy link
Copy Markdown
Contributor

Oh and also could you fix/rebase the branch to fix the merge conflicts? Thanks in advance!

@mkm17 mkm17 force-pushed the issues/6203_spp_autofillcolumn-apply branch from 81f60aa to c5ddbc6 Compare November 8, 2025 21:09
@mkm17 mkm17 force-pushed the issues/6203_spp_autofillcolumn-apply branch from c5ddbc6 to 346b519 Compare November 8, 2025 21:24
@mkm17
Copy link
Copy Markdown
Contributor Author

mkm17 commented Nov 8, 2025

@martinlingstuyl thank you for the review. I hope that I have covered all the suggestions.

@mkm17 mkm17 marked this pull request as ready for review November 8, 2025 21:29
@Adam-it
Copy link
Copy Markdown
Member

Adam-it commented Jan 6, 2026

@martinlingstuyl are you planning to push this PR forward?
This PR is blocking a follow up issue.
Let me know if you would like me to take this one over

@martinlingstuyl
Copy link
Copy Markdown
Contributor

Hi @Adam-it, I'm unable to pick it up currently, unfortunately... would you pick it up for me?

@Adam-it Adam-it self-assigned this Jan 8, 2026
@Adam-it
Copy link
Copy Markdown
Member

Adam-it commented Jan 8, 2026

On it

@Adam-it Adam-it merged commit 89db5fe into pnp:main Jan 26, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

New command: m365 spp autofillcolumn set

4 participants