Skip to content

Allow to define permissions for each datalayer instead of for the whole map - #1307

Merged
yohanboniface merged 40 commits into
masterfrom
datalayer-editstatus
Sep 22, 2023
Merged

Allow to define permissions for each datalayer instead of for the whole map#1307
yohanboniface merged 40 commits into
masterfrom
datalayer-editstatus

Conversation

@yohanboniface

@yohanboniface yohanboniface commented Sep 7, 2023

Copy link
Copy Markdown
Member

Work in progress!

cf #584

Todo list:

  • Create DataLayer.edit_status and expose in the API
  • adapt can_edit logic
  • allow to edit edit_status in the frontend
  • take it into account to allow or not editing feature
  • Expose only editable layers for new feature / editing feature's layer
  • make it work with anonymous maps
  • populate DataLayer.edit_status values from Map.edit_status
  • hide frontend map forms to users that do not have right to save
  • only show buttons create/delete datalayers to users with "map" access level
  • remove Map.edit_status from frontend/API
  • delete Map.edit_status (certainly not in this PR just in case)
  • backend tests
  • adapt frontend tests
  • integration tests (playwright)
  • decide if we keep both OWNER and EDITORS modes at datalayer level (this distinction does not exist anymore at map level)
  • naming (allowEdit / canEdit / allowMapEdit, hasDirty / isDIrty)
  • check delete map button for editors
  • check context menu
  • set default value for Datalayer.options.editMode
  • add unittests for editMode value returned by back-office (it's json dumped in a <script> in a HTML page, so it's better to use integration tests, which test the behaviour instead of the key itself, and are already done)

Comment thread umap/forms.py

DEFAULT_LATITUDE = settings.LEAFLET_LATITUDE if hasattr(settings, "LEAFLET_LATITUDE") else 51
DEFAULT_LONGITUDE = settings.LEAFLET_LONGITUDE if hasattr(settings, "LEAFLET_LONGITUDE") else 2
DEFAULT_LATITUDE = (

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Black noise :(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
DEFAULT_LATITUDE = (
DEFAULT_LATITUDE = getattr(settings, "LEAFLET_LATITUDE", 51)

Even shorter 😎

@yohanboniface
yohanboniface marked this pull request as draft September 7, 2023 08:37
@yohanboniface
yohanboniface force-pushed the datalayer-editstatus branch 3 times, most recently from cdbb5fa to e3cf017 Compare September 12, 2023 09:38
@yohanboniface yohanboniface changed the title WIP: move edit_status from Map to DataLayer Allow to define a different edit status for each DataLayer Sep 19, 2023
Object.defineProperty(this, 'isDirty', {
get: function () {
return isDirty || this.dirty_datalayers.length
return isDirty

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not sure why this check on dirty_datalayers was here before. As I can see, the isDirty status should always be set when a datalayer is made dirty…

if (!isDirty && status) self.fire('isdirty')
isDirty = status
self.checkDirty()
this.checkDirty()

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not sure this change is harmless. It works in our browser, but may it break on some older versions ?

@yohanboniface
yohanboniface marked this pull request as ready for review September 20, 2023 08:48
@yohanboniface yohanboniface changed the title Allow to define a different edit status for each DataLayer Allow to define permissions for each datalayer instead of for the whole map Sep 21, 2023
yohanboniface and others added 24 commits September 22, 2023 17:30
And only save the map in case of an `advanced` `editMode`.
Revert "Fix existing permissions related tests"

This reverts commit 36d7d87.

WIP
They now differ from the Map.edit_status ones
In some places we need to know if a given datalayer can accept new
features, or not, whether because being readonly or being remote
This bug has been introduced with this change:

8b4842f

That was not the correct fix, and this one should be the proper one.

We don't want to edit the permissions reference until we do save, otherwise user
cannot save as it is already no more the owner.

So:
- change permissions.options
- save
- commit those changes to map.options.permissions
- use only those values to check for isOwner and isAnonymousMap
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