Metro-Records / la-metro-councilmatic

:metro: An instance of councilmatic for LA Metro
MIT License
6 stars 2 forks source link

Add delete button to event detail pages #692

Closed shrayshray closed 2 years ago

shrayshray commented 3 years ago

There are two Finance & Budget meetings showing on the site. The one occurring at 1:30pm is the correct one. Please remove the one scheduled for 1:00pm. It has been removed in Legistar. image

hancush commented 3 years ago

Done. This has come up several times. I wonder if we should consider adding a delete button to event pages that is only visible to admin users, the same way you can upload an agenda if necessary. Thoughts, @shrayshray?

shrayshray commented 3 years ago

@hancush yes! Having a delete button in admin view would be amazing!

hancush commented 3 years ago

Task: Add a delete button visible only to admin users to the event detail page.

Context:

@fatima3558 With this in mind, want to do some code spelunking and propose how you'll implement this feature? Bullet points are fine!

fgomez828 commented 3 years ago

I'm taking a look at the code that currently exists on the event detail page, and it looks to me like there's already logic to allow an admin to delete an agenda if the agenda_url is not present.

The agenda_url is available in the template's context if the document note reads "Agenda", which seems to only happen if the agenda was uploaded automatically (based on the document notes in the other branches of the conditional). I'm not sure where this gets set.

Is my understanding of this right, @hancush? If there's already logic in place to do this, then there seems to be a bug in place that is preventing this button from showing up.

hancush commented 3 years ago

@fatima3558 No, at a high level, the logic is, if there is not a scraped agenda, show the button. Granted, it's implicit and can certainly be improved (though that is outside the scope of this PR).

The block you linked to is conditional on has_agenda being True, and agenda_url being False-y.

Here's how has_agenda is set:

https://github.com/datamade/la-metro-councilmatic/blob/1137e4b3e4cae435d8a81bc14342cfd54f543bbb/lametro/views.py#L204-L206

And here's how the other agenda URL context variables are set:

https://github.com/datamade/la-metro-councilmatic/blob/1137e4b3e4cae435d8a81bc14342cfd54f543bbb/lametro/views.py#L175-L184

I'd recommend not getting too bogged down in the agenda upload logic. I only meant to offer it as an example to show how you can hide/show things based on whether a user is authenticated. The other relevant portion is this block, which checks whether Legistar is available:

https://github.com/datamade/la-metro-councilmatic/blob/1137e4b3e4cae435d8a81bc14342cfd54f543bbb/lametro/views.py#L159-L161

Then, we advise whether or not to use the PDF upload based on the result:

https://github.com/datamade/la-metro-councilmatic/blob/1137e4b3e4cae435d8a81bc14342cfd54f543bbb/lametro/templates/lametro/event.html#L283-L287

A check like this that looks at the event's API source links could be a good way to only show the delete button for events that are no longer in the Legistar API (otherwise, they will just be re-scraped and appear again).

Does all of this make sense?

fgomez828 commented 3 years ago

@hancush gotcha, yes, makes sense. I was confusing what event and agenda mean for this specific issue.

Here's what I propose to do, then:

hancush commented 3 years ago

@fatima3558 Sounds like a plan to me. Deleting an event should delete its related objects, but you can check out the on_delete behavior in the python-opencivicdata models to be sure. Will you report back what you find?

fgomez828 commented 3 years ago

@hancush great, and yes, it looks like the python-opencivicdata EventAgendaItem does get deleted if the related Event gets deleted, here's the relevant code.

fgomez828 commented 3 years ago

hi @shrayshray! We are ready to have you test this event delete button. Here are the steps you’ll need to take:

  1. Create an event with a location of TEST in Legistar
  2. Wait half an hour so that the scrapers are able to import the data, then delete the test event from Legistar
  3. Log into the staging site, and go to the page for the event you created and deleted in Legistar. Make sure you see a button to delete the event.
  4. Click the delete event button and make sure the event is no longer present in the list of upcoming events

Let us know when you plan to start this test and how it goes when it’s done!

shrayshray commented 3 years ago

@fatima3558 the 3/1 meeting was just deleted in Legistar, so I'm going to try out the new delete button on the staging site now!

shrayshray commented 3 years ago

@fatima3558 The button was showing as expected on the event page ... and it deleted the event as expected! Success! This is super exciting, Fatima, thank you!!

hancush commented 3 years ago

snoopy celebrates

fgomez828 commented 3 years ago

awesome! closing this out as it's now deployed 🎉

hancush commented 3 years ago

@fatima3558 Let's keep this open and move this to done so we can track it as work done during this maintenance cycle.

shrayshray commented 3 years ago

@fatima3558 @hancush Today I visited the boardagendas.metro.net and there was a Regular Board Meeting showing up on the right side on the homepage under "Today's Meetings", for 10/1 at 10am (concluded): image

(Today is a Friday and there was not a Regular Board Meeting today). I checked in Legistar and no event for 10/1 at 10am exists, so I assume this was created by mistake and Omar and I were not notified when it was deleted. When I log into the site to delete it in Admin mode, I don't see the delete button: image

hancush commented 3 years ago

This is interesting. The delete button will only show if the event source URL no longer exists in the Legistar API. As it happens, the source links do exist, and they refer to the Spanish meeting of the 10/7 Operations, Safety, and Customer Service Committee.

Stashing how I checked the sources:

>>> from lametro.models import LAMetroEvent
>>> event = LAMetroEvent.objects.get(slug='regular-board-meeting-61a0f423297d')
>>> event
<LAMetroEvent: Regular Board Meeting (2021-10-01T17:00:00+00:00)>
>>> event.sources.all()
<QuerySet [<EventSource: http://webapi.legistar.com/v1/metro/events/2182>, <EventSource: https://metro.legistar.com/MeetingDetail.aspx?LEGID=2182&GID=557&G=A5FAA737-A54D-4A6C-B1E8-FF70F765FA94>]>

@shrayshray Is it possible that an existing event in Legistar was repurposed?

shrayshray commented 3 years ago

@hancush Yes, it is possible the 10/1 Regular Board Meeting was created then repurposed as the 10/7 Special Operations Meeting

hancush commented 3 years ago

Interesting, @shrayshray, thank you! With that in mind, maybe it makes sense to extend our logic for whether to show the delete button:

  1. The event source does not exist in the Legistar API (Current); OR
  2. The existent event is of a different meeting body (New condition)
    • If the meeting body stays the same, changes to date/time should be registered as updates and reflected on the board agendas site, so no reason to show the delete button in that case
    • If the meeting body is different, then a new event for that meeting body should have been created. To be extra safe, we could extend this logic one step further to confirm that's the case, though it's probably important to remove incorrect info independently of whether correct info has been created separately, so I lean no on this.

What do you think?

shrayshray commented 3 years ago

@hancush that looks good to me.

hancush commented 3 years ago

@fatima3558 I noticed the delete button is showing for logged in users for all regular board meetings on the staging site.

Screen Shot 2021-10-27 at 2 26 01 PM

Here's what a board meeting looks like in the API: http://webapi.legistar.com/v1/metro/events/2103

Based on the logic you implemented and the way a board meeting is represented in the API, can you tell me why the button is always showing?

fgomez828 commented 3 years ago

@hancush ah, good catch. The Legistar event has a more verbose name than the event as we have it in the database. In general, when Legistar event names are different from events in our database, are Legistar event names longer? If so, I would update the logic to check if the name of the event as stored in the database exists within the name of the event from legistar.

hancush commented 3 years ago

Good question! Want to check out the events scraper and see what transformation we apply to event names?

hancush commented 2 years ago

@shrayshray Behold! If you're logged in, you can now delete the errant board meeting from 10/1: https://boardagendas.metro.net/event/regular-board-meeting-61a0f423297d/

shrayshray commented 2 years ago

Woohoo! Goodbye, errant board meeting! Thank you @hancush !