Enhances spo page section add with collapsibleTitle option. Closes #6461
#6471
Conversation
|
Thanks @mkm17, we'll try to review it ASAP! |
There was a problem hiding this comment.
Let's change a few things before we merge it.
Just like you mentioned in your PR description we should indeed check for a deep equal of the last stubbed request. In the past, we sometimes implemented things differently, but over the years you learn how to make things better. Therefore we always ask our contributors to use the latest approach when adding new code.
| @@ -904,6 +904,42 @@ describe(commands.PAGE_SECTION_ADD, () => { | |||
| assert.strictEqual(data, JSON.stringify({ "CanvasContent1": "[{\"displayMode\":2,\"position\":{\"zoneIndex\":1,\"sectionIndex\":1,\"sectionFactor\":12,\"layoutIndex\":1},\"emphasis\":{},\"zoneGroupMetadata\":{\"type\":1,\"isExpanded\":false,\"showDividerLine\":false,\"iconAlignment\":\"left\"}},{\"controlType\":0,\"pageSettingsSlice\":{\"isDefaultDescription\":true,\"isDefaultThumbnail\":true}}]" })); | |||
| }); | |||
|
|
|||
| it('adds a OneColumn section at the end to an uncustomized page with collapsible setting and section title', async () => { | |||
| sinon.stub(request, 'get').callsFake(async (opts) => { | |||
| if ((opts.url as string).includes(`/_api/sitepages/pages/GetByUrl('sitepages/home.aspx')?$select=CanvasContent1,IsPageCheckedOutToCurrentUser`)) { | |||
There was a problem hiding this comment.
Let's check for the absolute URL instead of includes.
|
|
||
| let data: string = ''; | ||
| sinon.stub(request, 'post').callsFake(async (opts) => { | ||
| if ((opts.url as string).includes(`/_api/sitepages/pages/GetByUrl('sitepages/home.aspx')/savepage`)) { |
There was a problem hiding this comment.
Let's check for the absolute URL instead of includes.
| sinon.stub(request, 'post').callsFake(async (opts) => { | ||
| if ((opts.url as string).includes(`/_api/sitepages/pages/GetByUrl('sitepages/home.aspx')/savepage`)) { | ||
| data = JSON.stringify(opts.data); | ||
| return {}; |
There was a problem hiding this comment.
| return {}; | |
| return; |
| collapsibleTitle: 'Collapsible section title' | ||
| } | ||
| }); | ||
| assert.strictEqual(data, JSON.stringify({ "CanvasContent1": "[{\"displayMode\":2,\"position\":{\"zoneIndex\":1,\"sectionIndex\":1,\"sectionFactor\":12,\"layoutIndex\":1},\"emphasis\":{},\"zoneGroupMetadata\":{\"type\":1,\"isExpanded\":false,\"showDividerLine\":false,\"iconAlignment\":\"right\",\"displayName\":\"Collapsible section title\"}},{\"controlType\":0,\"pageSettingsSlice\":{\"isDefaultDescription\":true,\"isDefaultThumbnail\":true}}]" })); |
There was a problem hiding this comment.
Instead of checking it this way, let's use a deep equal on postStub.lastCall.
There was a problem hiding this comment.
Let's add the new option to the example where a collapsible section is created.
|
@milanholemans Thank you for the confirmation. I just wanted to double-check if these changes are also needed. I appreciate it, and I will try to make the changes promptly. |
mkm17
force-pushed
the
issues/6461_collapsible_section_title
branch
from
4d2fa5b to
4a1e63c
Compare
|
Hi @milanholemans, I have added the requested changes to the entire test file (not just the new code). I hope that is ok. And of course I have included a new example in the documentation. |
Wow, thanks for the commitment. We really appreciate it! |
Closes #6461
Let me know if there are any comments or changes needed.
In other commands, I recall there was a suggestion to make some changes in the tests, specifically to check
stubPost.lastCall.args[0].datainstead of directly comparing the data in this styleassert.strictEqual(data, JSON.stringify(...)