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

Additional changes for fix #498 #562

Merged
merged 3 commits into from
Jan 12, 2022

Conversation

fzbm
Copy link
Contributor

@fzbm fzbm commented Jan 3, 2022

The previously made changes for fix #498 were missing changes required for provisioning the header information.

Additional changes:

  • Removed case for HeaderLayoutType.Standard in ExtractObjects, because default already handles this.
  • Removed scope variable, because it is not used.
  • Removed the Emphasis type reference from Enum.TryParse, because the out variable already specifies the type.
  • ProvisionObjects now exits as soon as possible if no changes are required.
    • This also moves a possible server call for loading the web URL to a state, were this information would be required.
  • Introduced some (single) blank lines to increase the readability of the code.

The previously made changes for fix pnp#498 were missing changes required
for provisioning the header information.

Additional changes:

* Removed case for "HeaderLayoutType.Standard" in "ExtractObjects",
  because "default" already handles this.
* Removed "scope" variable, because it is not used.
* Removed the "Emphasis" type reference from "Enum.TryParse", because
  the "out" variable already specifies the type.
* "ProvisionObjects" now exits as soon as possible if no changes are
  required. This also moves a possible server call for loading the web
  URL to a state, were this information would be required.
* Introduced some (single) blank lines to increase the readability of
  the code.
czullu pushed a commit to MondayCoffee/pnpframework that referenced this pull request Jan 7, 2022
@czullu czullu mentioned this pull request Jan 7, 2022
@PedroMordeP
Copy link
Contributor

PedroMordeP commented Jan 11, 2022

Hi
I was looking at this pull request and at the line 76 on ProvisionObjects
web.EnsureProperties(w => w.Url);
should be changed to
web.EnsureProperties(w => w.HeaderEmphasis, w => w.HeaderLayout, w => w.MegaMenuEnabled);

??

I was working on this issue and I also made same changes like you and then found this pull request.

@PedroMordeP
Copy link
Contributor

If we don't do the web.EnsureProperties(w => w.HeaderEmphasis, w => w.HeaderLayout, w => w.MegaMenuEnabled);

I was getting this issue
The property or field 'HeaderLayout' has not been initialized. It has not been requested or the request has not been executed. It may need to be explicity request.

and the w.Url is not used :)

fzbm added 2 commits January 12, 2022 06:53
In some cases the "HeaderLayout" property will not be set and cause a
"PropertyOrFieldNotInitializedException" when accessing it later on.
Loading the existing value fixes this and will (in those cases) apply
the already set value again.

Loading the web URL is also not required, because "ExecutePostAsync"
will ensure it again.
Even if "ExecutePostAsync" ensures the property itself, loading the web
URL beforehand will save us an additional round trip to the server.
@fzbm
Copy link
Contributor Author

fzbm commented Jan 12, 2022

@PedroMordeP You are right. I also stumbled up on this exception during previous tests. A temporary fix is to load the web or specifically the HeaderLayout property before applying the template. I changed this and created an additional commit. A test of the change worked just fine. I applied a template to a freshly created ClientContext and PnP changed the header layout type as expected.

Also loading HeaderEmphasis and MegaMenuEnabled is not required, because HeaderLayout is the only property which might not get set. The two other will definitely be set before accessing the value.

ExecutePostAsync requires the web URL. While the method ensures the property itself, loading it beforehand will save us an additional round trip to the server.

jansenbe added a commit that referenced this pull request Jan 12, 2022
@jansenbe jansenbe merged commit f7c498e into pnp:dev Jan 12, 2022
@fzbm fzbm deleted the bugfix/498-missingheaderlayouttype branch January 12, 2022 09:28
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.

Missing HeaderLayoutType
3 participants