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

Make Page Publish respect Version Settings and Checkout Status on publish #361

Merged
merged 6 commits into from
Mar 29, 2021

Conversation

czullu
Copy link

@czullu czullu commented Mar 17, 2021

Publish does now check Versioning settings on SitePages Library and act's for Checkin and Publishing according to these settings.

Did not implement Approve on File as i hope there is a better way then setting _ModerationStatus=0 on ListItem.

@czullu
Copy link
Author

czullu commented Mar 19, 2021

@jansenbe To implement File Approve - is it the idea to take the request from CSOM like this
// Microsoft.SharePoint.Client.File
[Remote]
public void Approve(string comment)
{
ClientRuntimeContext context = base.Context;
ClientAction query = new ClientActionInvokeMethod(this, "Approve", new object[1]
{
comment
});
context.AddQuery(query);
}
and kind of replicate this in PnP.Core.Model.SharePoint.File? If so i can improve this pull request and implement Approve on File.

@codecov-io
Copy link

codecov-io commented Mar 20, 2021

Codecov Report

Merging #361 (cba0ca6) into dev (c13436b) will increase coverage by 2.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #361      +/-   ##
==========================================
+ Coverage   75.32%   77.35%   +2.02%     
==========================================
  Files         257      298      +41     
  Lines       19708    20967    +1259     
==========================================
+ Hits        14846    16220    +1374     
+ Misses       4862     4747     -115     
Impacted Files Coverage Δ
...del/SharePoint/Core/Internal/FieldLocationValue.cs 0.00% <0.00%> (-35.49%) ⬇️
...h/Public/UsernamePasswordAuthenticationProvider.cs 68.23% <0.00%> (-25.89%) ⬇️
.../Model/Teams/Internal/TeamChatMessageCollection.cs 69.92% <0.00%> (-25.00%) ⬇️
...P.Core/QueryModel/Query/BaseDataModelExtensions.cs 38.88% <0.00%> (-23.07%) ⬇️
.../Model/Teams/Internal/TeamChatMessageAttachment.cs 85.71% <0.00%> (-14.29%) ⬇️
...rc/sdk/PnP.Core/Services/Core/Query/QueryClient.cs 75.50% <0.00%> (-10.35%) ⬇️
...c/sdk/PnP.Core/Services/Core/Query/TokenHandler.cs 75.62% <0.00%> (-10.00%) ⬇️
...sdk/PnP.Core/Model/Base/BaseDataModelCollection.cs 56.52% <0.00%> (-7.44%) ⬇️
....Core/QueryModel/Query/DataModelQueryTranslator.cs 77.61% <0.00%> (-7.11%) ⬇️
.../PnP.Core/QueryModel/Query/ExpressionExtensions.cs 94.44% <0.00%> (-5.56%) ⬇️
... and 124 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 267607f...cba0ca6. Read the comment docs.

@czullu
Copy link
Author

czullu commented Mar 21, 2021

@jansenbe i took File Checkin as Template and implement the File.Approve same way.

@jansenbe jansenbe self-assigned this Mar 23, 2021
@jansenbe jansenbe added the area: model 📐 Related to the core SDK models label Mar 23, 2021
@jansenbe
Copy link
Contributor

Thanks @czullu , will process this later today/tomorrow

@jansenbe
Copy link
Contributor

@czullu : This change will need unit tests to cover the new features / scenario. You want to work on that or do you want me to handle that?

@czullu
Copy link
Author

czullu commented Mar 23, 2021

@czullu : This change will need unit tests to cover the new features / scenario. You want to work on that or do you want me to handle that?
It might be a good moment to do the Test Config as in the Docu. I do tonight and provide the Unit Test for File.Approve.

@czullu
Copy link
Author

czullu commented Mar 26, 2021

@jansenbe Unit Tests now included for File Approve and multiple tests for Page Publish with different SitePages-Library settings for MinorVersion turned off/on, Moderation turned off/on, ForceCheckout off/on. Did try to optimize number of tests vs coverage as the underlying methods have tests by them self.

jansenbe added a commit that referenced this pull request Mar 29, 2021
@jansenbe jansenbe merged commit 38e5228 into pnp:dev Mar 29, 2021
@jansenbe
Copy link
Contributor

Really nice job with the tests, thanks @czullu 💪🥇

@czullu czullu deleted the Fix_PagePublish branch May 25, 2021 06:36
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

Successfully merging this pull request may close these issues.

3 participants