django-leaflet-storage icon indicating copy to clipboard operation
django-leaflet-storage copied to clipboard

MapManager (replaces PublicManager)

Open plepe opened this issue 9 years ago • 3 comments
trafficstars

I think the PublicManager is not an ideal solution. This pull request replaces PublicManager by a MapManager with a visible() filter which takes the request object as parameter. It adds additional filters, so that maps of the current user are also included. This also simplifies adding additional permissions.

plepe avatar Jan 09 '16 22:01 plepe

Purpose sounds OK, but tests are broken :s Also you may need to add more to tests to cover the new subtleties of the manager.

yohanboniface avatar Jan 10 '16 10:01 yohanboniface

Yes, I forgot about the tests. Will do soon.

I kept thinking about this change, and now I'm not so sure if I should remove the PublicManager, because it might break other code (I don't know if there are other users of leaflet_storage). Maybe deprecate PublicManager and print a warning if it is being used?

Btw, there's a visible change: the front page and the search page now include maps which are non-public but visible by the current user.

Talking about visibilities: I think the "My Maps" page only shows maps where the current user is owner. Shouldn't this page also include maps where the current user is editor?

plepe avatar Jan 10 '16 12:01 plepe

I kept thinking about this change, and now I'm not so sure if I should remove the PublicManager, because it might break other code (I don't know if there are other users of leaflet_storage). Maybe deprecate PublicManager and print a warning if it is being used?

I feel like we can make this change. This will only change the behaviour for logged in users, and it will only show more maps that this user is allowed to see, so that's fine.

Talking about visibilities: I think the "My Maps" page only shows maps where the current user is owner. Shouldn't this page also include maps where the current user is editor?

Yes, but only if map edit_status allow editors (or everyone). I.e. not in the case where someone is editor of a map, but the edit_status only allow the owner to edit it.

yohanboniface avatar Jan 22 '16 12:01 yohanboniface