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

567 simple #613

Merged
merged 7 commits into from
Jan 25, 2022
Merged

567 simple #613

merged 7 commits into from
Jan 25, 2022

Conversation

tarjeieo
Copy link
Member

@tarjeieo tarjeieo commented Dec 30, 2021

Your checklist for this pull request

  • Make sure you are requesting to pull a topic/feature/bugfix branch (right side). Don't request your main!
  • Make sure you are making a pull request against the dev branch (left side). Also you should start your branch off dev.
  • Check the commit's or even all commits' message
  • Check if your code additions will fail linting checks
  • Remember: Add issue description to CHANGELOG with the ID of the Issue associated with this PR
  • Documentation: Have a look at the PP365 User manual and consider the need for updates to be made. Updates can be done directly into the 'Kladd' branch or by providing information to test team for implementation.

Description

Skipping applying navigation if it's an upgrade

How to test

Please describe how someone else (a regular user without coding skills) can test this PR. In the form of a numbered list and checkboxes for Jan when the testing period occurs.

Example:

  • Upgrade a pre 1.2.7 install
  • Upgrade a post 1.2.7 install
  • Observe that the portfolio navigation is equal in the two cases

Update of documentation

  • Check if user manual requires update

Relevant issues (if applicable)

#567

💔Thank you!

@tarjeieo tarjeieo added this to the 1.3.2 milestone Dec 30, 2021
@tarjeieo tarjeieo requested a review from olemp as a code owner December 30, 2021 14:52
@tarjeieo tarjeieo self-assigned this Dec 30, 2021
@tarjeieo tarjeieo marked this pull request as draft January 14, 2022 12:12
@tarjeieo
Copy link
Member Author

Going back and forth here... With this change, we are not adding new navigation nodes to the top navigation bar on upgrade, nor are we resetting the navigation.

It's of course possible to write an upgrade script that adds "missing" items to the navigation, but what if the customer doesn't want this functionality? Then it's always added back in case of an upgrade - which is not what we want either.

I'm leaving this PR more or less as-is, only adding an output to the install script that the project timeline page needs to be added to the navigation manually.

@tarjeieo
Copy link
Member Author

Changed my mind - again! I'm adding the navigation node IF and only if the already installed version is pre-1.2.7. This should handle this in the better way IMO.

@tarjeieo tarjeieo marked this pull request as ready for review January 16, 2022 15:47
@tarjeieo tarjeieo requested a review from okms January 16, 2022 15:47
@tarjeieo tarjeieo changed the base branch from main to dev January 16, 2022 15:48
@okms
Copy link
Contributor

okms commented Jan 16, 2022

We need to remember to backport this to #421

Copy link
Contributor

@okms okms left a comment

Choose a reason for hiding this comment

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

Looks good. Note that SupportedUILanguages appears to do nothing when using PnP.PowerShell, so for Schema 2020/2 and newer this has no effect from my limited testing.

@tarjeieo tarjeieo merged commit ea70634 into dev Jan 25, 2022
@tarjeieo tarjeieo deleted the 567-simple branch January 25, 2022 09:29
@pzljanb pzljanb mentioned this pull request Jan 31, 2022
19 tasks
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.

2 participants