Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TECH DEBT] MediaSourceService defined but never used #2298

Open
kayakr opened this issue Apr 22, 2024 · 3 comments
Open

[TECH DEBT] MediaSourceService defined but never used #2298

kayakr opened this issue Apr 22, 2024 · 3 comments
Labels
Type: bug identifies a problem in the software with clear steps to reproduce

Comments

@kayakr
Copy link
Contributor

kayakr commented Apr 22, 2024

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.

@kayakr kayakr added the Type: bug identifies a problem in the software with clear steps to reproduce label Apr 22, 2024
@ajstanley
Copy link
Contributor

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
Copy link
Contributor Author

kayakr commented Apr 23, 2024

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
Copy link
Contributor Author

kayakr commented Apr 23, 2024

@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...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: bug identifies a problem in the software with clear steps to reproduce
Projects
None yet
Development

No branches or pull requests

2 participants