Add delete route for map groups and refine existing group URLs#28
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates Django routing for map group management within the wcoa app by adding an explicit delete endpoint and tightening the URL patterns for the group listing views to avoid over-matching.
Changes:
- Added a delete route for map groups that routes to
DeleteMapGroupActionViewwithpkandslug. - Updated group listing routes to only match exact
/collaborate/groups/and/groups/URLs (with optional trailing slash).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| re_path(r'^collaborate/groups/?(?P<pk>\d+)/(?P<slug>[\w-]+)/delete/?$', groupviews.DeleteMapGroupActionView.as_view(), name='delete'), | ||
| re_path(r'^collaborate/groups/?(?P<pk>\d+)/(?P<slug>[\w-]+)/edit/?', views.show_wcoa_edit_mapgroups, name='edit'), | ||
| re_path(r'^collaborate/groups/?(?P<pk>\d+)/(?P<slug>[\w-]+)/preferences/?$', MapGroupPreferencesView.as_view(), name='preferences'), | ||
| re_path(r'^collaborate/groups/?(?P<pk>\d+)/(?P<slug>[\w-]+)/?$', views.show_wcoa_detail_mapgroups, name='detail'), |
| re_path(r'^collaborate/groups/?(?P<pk>\d+)/(?P<slug>[\w-]+)/?$', views.show_wcoa_detail_mapgroups, name='detail'), | ||
| re_path(r'^collaborate/groups/?', views.show_wcoa_mapgroups, name='show_wcoa_mapgroups'), | ||
| re_path(r'^groups/?', views.show_wcoa_mapgroups, name='show_wcoa_mapgroups'), | ||
| re_path(r'^collaborate/groups/?$', views.show_wcoa_mapgroups, name='show_wcoa_mapgroups'), |
There was a problem hiding this comment.
Was there a strong need to enforce URL end here with $? I can see tradeoffs with adding it:
- Pro: potential future URL flow confusion can be avoided, especially highlighting typos
- Con: less elegant failure - goes to error instead of failing gracefully back to the (collaborate/)groups/page.
There was a problem hiding this comment.
Ah! I see how this interplays with the new line you added: if the PK or the slug does not exist, then the DELETE API instead might return someone (depending on how mismatches are handled) to the Groups page without an error. I can see that being confusing
Add a new route for deleting groups and correcting the patterns for listing groups.
Group management improvements:
DeleteMapGroupActionViewwith bothpkandslugparameters.URL matching fixes:
show_wcoa_mapgroupsview is only matched on exact URLs (/collaborate/groups/and/groups/), preventing accidental matches with more specific group URLs.