Islandora / documentation

Contains islandora's documentation and main issue queue.
MIT License
104 stars 73 forks source link

[TECH DEBT] MediaSourceService defined but never used #2298

Open kayakr opened 2 months ago

kayakr commented 2 months ago

What steps does it take to reproduce the issue? https://github.com/Islandora/islandora/blob/2.x/src/Form/ConfirmDeleteMediaAndFile.php#L60 form declares need for MediaSourceService and stores it locally but AFAICT never uses that variable.

We can remove the code referencing MediaSourceService.

ajstanley commented 2 months ago

We might want to go one step further by moving the media and file delete function into utils. We have almost identical code here and here.

Further to this there is code in there to delete the translations, but my reading (verified by some experimentation) is that media->delete() and $node->delete() calls drop the translations on their own. Has anyone out there seen cases where translations can linger after a media and file delete?

kayakr commented 2 months ago

We might want to go one step further by moving the media and file delete function into utils. We have almost identical code here and here.

I agree. I dislike having such logic in a form submit handler as the same logic defined as a service or similar could be invoked programatically as part of a form submission, or a script, or via API, etc. And having the same logic in two distinct submitForms violates DRY.

kayakr commented 2 months ago

@ajstanley My suggestion is to create a deleteRepositoryItemsAndMediaAndFiles() function in Islandora Utils can combine the code from the two examples you mention. I can make a start if you want...