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

Read-only access for project list webpart #498

Merged
merged 52 commits into from
Oct 14, 2021
Merged

Read-only access for project list webpart #498

merged 52 commits into from
Oct 14, 2021

Conversation

Rundez
Copy link
Contributor

@Rundez Rundez commented Aug 9, 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 PR description to CHANGELOG with the ID that matches this PR

Description

Added the possibility to see all projects when the user is in the "Porteføljeinnsyn" group. This can be toggled on and off in the projectlist web part. If it is toggled on, the user(s) in "Porteføljeinnsyn" will notice additional buttons appear right of the search bar. These buttons will change the current view, toggling between seeing all projects, and projects that the user is a member of.

How to test

  1. Add a user to the "Porteføljeinnsyn" group.
  2. Navigate to the project list property pane and turn on "Vis alle prosjekter"
  3. Toggle the buttons that appear. No deleted projects should appear, and projects you are not a member of should not ble clickable.

Disclaimer:
Deleted projects will be shown if "All projects" is selected. A solution is will be delivered in a separate PR.

Relevant issues (if applicable)

closes #443

💔Thank you!

@Rundez Rundez requested a review from olemp August 9, 2021 10:27
@Rundez Rundez self-assigned this Aug 9, 2021
@olemp
Copy link
Collaborator

olemp commented Aug 9, 2021

@Rundez Have you installed this somewhere for testing?

@Rundez
Copy link
Contributor Author

Rundez commented Aug 9, 2021

@olemp I have only served it in my own dev environment.

@tarjeieo
Copy link
Member

@Rundez can we setup a separate instance of pp365 in our demo tenant and share for testing? Seems like something that should be thoroughly tested

@Rundez
Copy link
Contributor Author

Rundez commented Aug 17, 2021

@tarjeieo I am setting one up.

@Rundez
Copy link
Contributor Author

Rundez commented Aug 18, 2021

Created a PP instance at this site.

@Rundez
Copy link
Contributor Author

Rundez commented Aug 18, 2021

Two projects are created. The user Hugo First have been added in the "Porteføljeinnsyn" group, and will then have the ability to see projects he's not member of.

@tarjeieo
@olemp

* Added opacity and alert message to ReadOnly projects (Frontpage)

* Removed alert message and added tooltip and cursor to ReadOnly projects

* Using title html attribute

Co-authored-by: Ole Martin Pettersen <olemp@puzzlepart.com>
@olemp olemp removed a link to an issue Sep 2, 2021
Copy link
Collaborator

@olemp olemp left a comment

Choose a reason for hiding this comment

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

Most looks good, but I don't understand the logic 100%.

What does the "Vis alle prosjekter" toggle do? And how is it related to "Flisevisning"?

image

Is it the toggle in web part properties that decides whether to show the "Mine prosjekter/Alle prosjekter" links? I don't find that so intuitive. Do we need a webpart property to toggle this option?

Also, we need to change the placeholder based on what "view" is selected.

image

@Rundez
Copy link
Contributor Author

Rundez commented Sep 2, 2021

Ah yes - it is not related to "Flisevisning" actually. As you said, it decides wether the links are displayed. It might be unnecessary, or it could be moved to another submenu in project properties.

I agree with the placeholders. As I removed the function that checks for deleted projects the problem occured. It did only occur on projects in the project list that actually is deleted? I will create a fix.

@Rundez
Copy link
Contributor Author

Rundez commented Oct 14, 2021

Having a little isuse. While testing at https://puzzlepart.sharepoint.com/sites/pp365.
Seems it seems like all projects, even those who already got a project logo is overwritten by the placeholder. Probably a minor thing which will be solved pretty fast.

loading: true,
searchTerm: '',
showAsTiles: props.showAsTiles,
onlyAccessProjects: true
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is onlyAccessProjects used like a prop @Rundez? Seems like it's hardcoded to true? Don't see the point of this state variable, but I might be missing something.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's meant to toggle between what projects are shown. I realize it might need a namechange for readability...

image

Copy link
Collaborator

Choose a reason for hiding this comment

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

@Rundez My Projects will only show projects the user is a member of any way, so I don't see the need for this property?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It only does so because of the filtering done in _filterProjects though. As seen below:
(just did a namechange to displayAllProjects)
image

Copy link
Collaborator

Choose a reason for hiding this comment

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

How is readOnly set, and where?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is set in the constructor of the ProjectListModel.

Used in the _mapProjects function in data/index.tsx

@olemp olemp changed the title Read-only access to portfolio Read-only access for project list webpart Oct 14, 2021
)

if (!this.state.onlyAccessProjects)
projects = projects.filter((project) => project.readOnly === false)
Copy link
Collaborator

Choose a reason for hiding this comment

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

@Rundez Where is readOnly property set?

Copy link
Collaborator

@olemp olemp left a comment

Choose a reason for hiding this comment

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

See comments above.

@olemp olemp marked this pull request as draft October 14, 2021 10:18
@olemp
Copy link
Collaborator

olemp commented Oct 14, 2021

Having a little isuse. While testing at https://puzzlepart.sharepoint.com/sites/pp365. Seems it seems like all projects, even those who already got a project logo is overwritten by the placeholder. Probably a minor thing which will be solved pretty fast.

What is happening exactly? Seems to work here.

image

@Rundez
Copy link
Contributor Author

Rundez commented Oct 14, 2021

What is happening exactly? Seems to work here.

If it's over 20 projects, the project logo fetch is batched. It seems like that creates the issue. Not completely sure why it's happening though.

@olemp
Copy link
Collaborator

olemp commented Oct 14, 2021

What is happening exactly? Seems to work here.

If it's over 20 projects, the project logo fetch is batched. It seems like that creates the issue. Not completely sure why it's happening though.

This might solve the issue.

@Rundez
Copy link
Contributor Author

Rundez commented Oct 14, 2021

What is happening exactly? Seems to work here.

If it's over 20 projects, the project logo fetch is batched. It seems like that creates the issue. Not completely sure why it's happening though.

This might solve the issue.

Works like charm!

@olemp olemp marked this pull request as ready for review October 14, 2021 12:00
@olemp olemp merged commit 100d7d6 into dev Oct 14, 2021
@olemp olemp deleted the feature/#433 branch October 14, 2021 12:22
@pzljanb pzljanb mentioned this pull request Oct 15, 2021
26 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.

Own projects not selectable in Porteføljeoversikt Read-only access to portfolio frontpage project view
4 participants