Skip to content

Conversation

ClausRogisch
Copy link

@ClausRogisch ClausRogisch commented May 17, 2025

First i am sorry, that i am not following the guidlines. As i am fairly unexperienced with python.
This may be of interest.

Changed Team behaviour. Now Maps and Teams are in a ManyToMany relationship.
Further the DataLayerPermissions now allow for a finer controll. Editors and Teams can be connected to DataLayers.
Setting the edit_status to the new value, the permission logic allows edit rights to the specified editors and users in the associated teams.

Code review definetly needed.

TODO:
I guess changing the migrations, as the teams might get lost.
Could be starting point for #1892 in a simmilar fassion
as mentioned in issue #2707 the ui representation is still not working, making interaction unintuitive as a user is able to still click the features and open the edit dialogs, but saving fails.
image

…tus for finegrained edit controll for layers
@ClausRogisch ClausRogisch changed the title Permission Update feat: per layer permissions for editors and teams May 17, 2025
@yohanboniface
Copy link
Member

Hey @ClausRogisch thanks a lot for this huge work! And sorry for the late reply.
We wanted to discuss this a bit with the team.
We'd like to challenge the need, in comparison to the complexity added to the code and the UI.

Could you elaborate a bit more on your use case ? Also, if you have any idea of other users needing this (other issues or forum discussions) to help us weight correctly the need, please share.

the ui representation is still not working, making interaction unintuitive as a user is able to still click the features and open the edit dialogs, but saving fails.

This part should be fixed indeed anyway.

@Aurelie-Jallut @C-Sophie what are your thoughts on this feature ? In summary, this PR allows to restrict editing a layer to some given editor or some team. So it would be possible for example to have a layer only editable by team "SuperTeam" and another layer only editable by "Joe and Marianna". While today, we can only choose to restrict a layer to the owner, or to all the map editors and team, or to everybody. Also, this PR allows to attach more than one team to a map (and a layer).

@ClausRogisch
Copy link
Author

Hey @yohanboniface,
i will have a look into possibly related issues on sunday.
My motivation:
I am taking part in an assocciation that plans a festival. There are a lot of people involved that are organized in teams.
For planning the site, i wanted a solution, that enables us to work collaboratively, without risking damage to the work and progress of other teams. so it, energy, security and alike are able to work in the same map, always having the information or current planning level of other teams at their hand, eliminating the need for inter teams plena/meetings. But as a lot of people are using the map, the layers that are controlled by example team it are safe from manipulation from the energy teams and other scenarios.
umap is easy to use and was almost there, hence i took it as a starting point, replacing the current workflow of a manually currated graphical siteplan.

As the current build implemented a layerbased permission model in a global way, it seems only natural to extend it opionally for more fine grained controll.

@yohanboniface
Copy link
Member

@ClausRogisch thanks for taking the time to detail your use case.

We are waiting for our biweekly meeting to discuss this in team a bit more, as we think it's not a trivial change, and it will certainly need work from us too, which needs to be prioritized/arbitrated.

A few thoughts on the process, it we decide to take this path, we'd suggest to do it in three steps (thus three PRs):

  • one first step to make team a m2m instead of only a FK (and adapting tests…)
  • one second step to add team and editors to the DataLayer model
  • one final step to make the UI reflect the permissions (so a user cannot edit a feature it they does not have the permission to do so, while currently this is not done and only the server is checking such permissions).

We'll get back to you at the end of this week hopefully with a better visibility.

Thanks again for your work!

@ClausRogisch
Copy link
Author

@yohanboniface what I did not have a look into is how the team management is organized and how the permissions are handled. (Whether there is a owner concept for a team, that can manage the members)
It is maybe a point to keep in mind for the discussion.

@ClausRogisch
Copy link
Author

Concering the conversion for the team to m2m, this might not be neccessary if the assignments are done following my comment on the issue
#2713 (comment)

keeping the current team realtion -> but as owner, so every team member is owner of the map.
and introducing the relations mentioned in the comment for the editor role

@yohanboniface
Copy link
Member

Hi @ClausRogisch and sorry again for my late answer!

Your proposal of using role based access control is interesting. I'd like to involve @Aurelie-Jallut (our UX designer) on this reflection. Let me get back to you as soon as we can (within two weeks, now we are a bit in a rush with the French SotM and other things).

@ClausRogisch
Copy link
Author

@yohanboniface
I am thankfull for your concern regarding the issue. And thank you for the updates. There is no rush for me, so you really don't have to excuse yourself for replying late.

I am working to the things I need in my fork, and try to involve your main repo on things I am changing in case it might be of interest for the main project.

I will try to answer questions and thoughts on these matters, but as it is most likely effort in spare time, there might be times with higher presence and times with fewer.

So if there are updates from your inner meetings or new questions I might answer, don't hesitate to reach out, but don't stress yourself with keeping me involved, only in order to show me that the issue is not forgotten.

@Aurelie-Jallut if you want to get acces to the current working instance, please reach out and I can set up a Testaccount in the currently used idp, so see the made changes for yourself.

As it is a kind of hack, the UX is not that good, I tried not to deviate to much from existing elements, but in big projects, the usability lacks a litte bit of overview

1 similar comment
@ClausRogisch
Copy link
Author

@yohanboniface
I am thankfull for your concern regarding the issue. And thank you for the updates. There is no rush for me, so you really don't have to excuse yourself for replying late.

I am working to the things I need in my fork, and try to involve your main repo on things I am changing in case it might be of interest for the main project.

I will try to answer questions and thoughts on these matters, but as it is most likely effort in spare time, there might be times with higher presence and times with fewer.

So if there are updates from your inner meetings or new questions I might answer, don't hesitate to reach out, but don't stress yourself with keeping me involved, only in order to show me that the issue is not forgotten.

@Aurelie-Jallut if you want to get acces to the current working instance, please reach out and I can set up a Testaccount in the currently used idp, so see the made changes for yourself.

As it is a kind of hack, the UX is not that good, I tried not to deviate to much from existing elements, but in big projects, the usability lacks a litte bit of overview

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