nova-flexible-content icon indicating copy to clipboard operation
nova-flexible-content copied to clipboard

File fields "delete" button returns 404

Open toonvandenbos opened this issue 6 years ago • 23 comments

The Delete button under a file field does not work correctly.

image

Nova responds with something like:

DELETE http://example.tld/nova-api/{resource}/{resourceId}/field/2011da1c6d582ce2__image 404 (Not Found)

This is because the fields inside a Flexible field do not exist in the resource's root fields.

toonvandenbos avatar Feb 25 '19 19:02 toonvandenbos

I looked at that issue quickly, the only solution I can think of is to have a FlexibleFile/FlexibleImage field that emulates the same behavior as the original File field but will allow us to change the controller actions called by the vue component. The issue with that solution is that we'll have to track and maintain the same level of functionality than the original Nova field, it's not frequently updated but still.

Does that makes sense or do you have any other idea? I could work on a PR in a few days, let me know!

raphcollective avatar Mar 21 '19 20:03 raphcollective

+1 Is this already fixed? I have the same issue and can't remove any images.

wimurk avatar Apr 19 '19 09:04 wimurk

Looks like something related is happening when you use a Trix field. Getting 404s when adding images to a Flexible layout containing a Trix field.

            Flexible::make('Content')
                ->addLayout('Simple content section', 'wysiwyg', [
                    Text::make('Title'),
                    Trix::make('Content')->withFiles('local')->alwaysShow()
                ]),

A normal Trix upload will post to (eg):

/nova-api/blocks/trix-attachment/content

whereas within a Flexible field, it's posting to:

/nova-api/blocks/trix-attachment/4dd76ad5528575d4__content

and getting a 404 response.

element25 avatar Apr 19 '19 10:04 element25

Any progress on this?

wimurk avatar May 09 '19 05:05 wimurk

I guess it's a complicated fix so does it make sense to update the docs to say that you can use a Trix field but not with images?

element25 avatar May 14 '19 21:05 element25

Does anyone have a solution for bypassing this issue?

rokde avatar Aug 12 '19 11:08 rokde

Hi all, Just checking for an update on this, I find it hard to believe on such a great tool this has not been fixed, unless we are doing something wrong. Basically one cannot remove and image without deleting the whole layout and that leaves the assets on disk.

I am looking into it, but code is quite complex. Did anyone sort this?

ajarti avatar Aug 29 '19 07:08 ajarti

@ajarti i tried so fix it myself but like you said the code is complex and hard to read.

i'm already looking for another package.

wimurk avatar Aug 29 '19 07:08 wimurk

@Nyratas Is there maybe a way we can also intercept this delete request in a middleware similar to what we have done for media library fields?

voidgraphics avatar Aug 29 '19 08:08 voidgraphics

@voidgraphics We could give it a try, but once again, that's a dirty workaround and I'm not sure we'll be able to fix it that way since it involves altering the behavior of an original Nova Controller.

For everyone's information, the file's delete route uses the resource's identifier and its field's attribute as parameters. This way, the controller will be able to find the file by resolving the field on the resource. If the file field is used inside a Flexible Content, the Nova Controller won't be able to find it (because it is not a root field of the resource) and returns an error. Believe me, this is not an easy issue.

@wimurk I agree, the code may sometimes be quite complex. Knowing it has multiple complex responsibilities in an environment that has not been built for its behavior, this may cause complex workarounds. However, it is not "hard to read" (in my opinion). Each method is documented, we use a clear naming convention, avoid method complexity as much as possible and do our best to implement SOLID principles wherever we can. I'm sure there's still room for enhancement, of course. Feel free to suggest code style changes if needed.

toonvandenbos avatar Aug 29 '19 09:08 toonvandenbos

Hi guys,

Just to clarify, "hard to read" not meaning it's not good code, it is, meaning it's hard to keep a mental model in one's head of all the moving parts and track each variables actual value etc.

If you have not developed a Nova plugin and have not got a handle on the inner workings is quite daunting.

Thanks again for a great tool.

ajarti avatar Aug 29 '19 09:08 ajarti

Yup, the issue lies here in FieldDestroyController in the handle method. As @Nyratas pointed out - when Nova tries to find the field by attribute it fails, because the field does not exist in the root.

$field = $resource->updateFields($request)
            ->whereInstanceOf(File::class)
            ->findFieldByAttribute($request->field, function () {
                abort(404);
            });

One of the solutions could be to overwrite the updateFields method on the resource by returning a custom FieldCollection instance which uses a different way to find fields by attribute. This could all be part of the package, you could just include a trait in your resource.

I might give it a go in the coming days.

On second inspection - overriding the availableFields method on the resource and applying a custom instance of the collection should do the trick.

slovenianGooner avatar Aug 31 '19 11:08 slovenianGooner

So what I've tried to do was to flatten the array of available fields on DELETE requests - but then I have an issue because the query does not go through for updating in the database.

I somehow get over the "findFieldByAttribute" method by flattening the array of fields - but then it stops for me. If anyone can help I'd be happy to share my code and what I've done so far.

I don't even want to get started on nested flexible content etc.

slovenianGooner avatar Aug 31 '19 12:08 slovenianGooner

@slovenianGooner can you create a merge request so we can pull from that branch.

wimurk avatar Sep 02 '19 06:09 wimurk

@wimurk available at https://github.com/whitecube/nova-flexible-content/pull/72

slovenianGooner avatar Sep 02 '19 07:09 slovenianGooner

Any updates on this?

ibrahim-mubarak avatar Aug 19 '20 15:08 ibrahim-mubarak

it looks like these commits were launched in v0.2.6, but the problem persists.

paperscissors avatar Oct 14 '20 18:10 paperscissors

@paperscissors this problem exists since the beginning and there has not been an attempt to fix this since. Nova's file fields still need to be "root" fields for this to work, which is not the case when they're used inside Flexible fields.

toonvandenbos avatar Oct 14 '20 18:10 toonvandenbos

any update about the issue ?

baskinvolkan avatar Mar 15 '21 12:03 baskinvolkan

Just ran into this. Seems like no progress is being made.

If possible you can disable downloads and deletions. In my case this works fine but Nova really needs to sort out its fields system:

->deletable(false)
->disableDownload()

stuartcusackie avatar Sep 08 '21 11:09 stuartcusackie

Still persists

dinmezpinar avatar Apr 26 '23 13:04 dinmezpinar

Still persists

SebastianBotez avatar Jul 19 '23 22:07 SebastianBotez

Still persists

LeonardoFraliniU2Y avatar Aug 08 '23 14:08 LeonardoFraliniU2Y