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

Fix broken references and user permission check in "react-company-templates" #1378

Merged
merged 8 commits into from
Apr 18, 2024

Conversation

tmaestrini
Copy link
Contributor

@tmaestrini tmaestrini commented Apr 6, 2024

Q A
Bug fix? yes
New feature? no
New sample? no

What's in this Pull Request?

This PR fixes three incorrect references due to copy-paste errors in the solution template used by the author. Moreover, it addresses an erroneous user permission check to set the correct visibility of the command and "prettifies" some declarations.
That said, it changes

  1. the solution id to ensure sure it won't clash with any solution id that is used in another sample extension
  2. the location of the action button of the command set
  3. the component reference of the action button to match the correct extension id used in the manifest
  4. the permission check for the user's permissions on the actual document library
    There was no security issue in this extension before – the current permission fix implements better user experience: it now hides the command button for any user that has no permission to add documents to the library.

Nevertheless, this update is necessary, otherwise the solution can cause side effects due to incorrect references when using it in production.

@tmaestrini
Copy link
Contributor Author

Hey @hugoabernier 👋
As I will present this sample in the Viva Connections and SPFx community call on 18 April, I would be very happy if this PR was merged before this deadline.
Do you think this would be possible?

@tmaestrini
Copy link
Contributor Author

Anyone down on this? Please let me know if I have to do anything else...

@hugoabernier
Copy link
Collaborator

Anyone down on this? Please let me know if I have to do anything else...

No, you don't need to do anything else but be patient. We usually process the PRs every other weekend.

@tmaestrini
Copy link
Contributor Author

Thanks for your support, @hugoabernier! 🙌

@hugoabernier hugoabernier merged commit a51b121 into pnp:main Apr 18, 2024
3 checks passed
@hugoabernier
Copy link
Collaborator

Thank you @tmaestrini for your update (and your patience)! Awesome!.

Thank you for sharing your sample with others - you rock! 👏🥇👩‍💻

@tmaestrini
Copy link
Contributor Author

Many thanks to you, @hugoabernier! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants