bug: destroy routes don't redirect

[DrumsnChocolate]

How to reproduce

Destroy a photo-comment

Expected behaviour

on successful destroy, redirect to photo album (according to implementation)

Desirable behaviour

Instead of redirecting to album, it makes more sense to redirect to photo.

Current behaviour

destroy succeeds, but no flashnotice is shown, and no redirect takes place.

Suspected reason

the refactor in which we removed mixins, where the model-save util was introduced, didn't properly couple the edit controller actions to the model save util methods (overriding the onSuccess action in the destroy controller does not change the actual behaviour in the modelSaveUtil class, because destroy inherits from edit, not from ModelSaveUtil). There are other issues related to passing modelSaveUtil.onSuccess to the then of a promise, but after fixing those, I found the root issue being reliance on inheritance where there is none.

[guidojw]

#495

[DrumsnChocolate]

I don't think that PR fixes the full problem described in this issue

[guidojw]

Oh I only read the title, will check

[guidojw]

Okay yeah the PR doesn't fix redirects, just the this is undefined error. Reading the error on Sentry I thought this was causing the broken redirects.

For the inheritance errors where things like successMessage are not correctly set, I think converting the controllers to Octane style will fix this.

For the inheritance errors where methods like onSuccess on the destroy controllers aren't being called, I think it's best to add a check in ModelSaveUtil if the entity has an onSuccess method and then call that instead, but for that to work we also need Octane style controllers.

[DrumsnChocolate]

I think that should do it yes. Nice idea to conditionally call entity.onSuccess

[DrumsnChocolate]

@guidojw is this still an open issue? I presume it is, since I encountered a similar problem in #547
Is this related to #539 ?

[guidojw]

Yeah I have not touched this. It is not related to #539

[DrumsnChocolate]

Okay, so I can start working on refactoring CRUD related controllers?

[guidojw]

Okay, so I can start working on refactoring CRUD related controllers?

#539 only updates (is gonna) components in app/components/model-form so that should be okay 👍🏿