Problem/Motivation

In #3401734: Refactor FileUploadResource to use FileUploadHandler we are trying to refactor out duplicate upload code by re-using FileUploadHandler. However, core file field uploads that currently use FileUploadHandler will rename to filename property of a File entity if

  1. There is a security rename
  2. There is an existing file with that name (using EXISTS_RENAME)

e.g. https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/file/... 

// Update the filename with any changes as a result of security or renaming
// due to an existing file.
$file->setFilename($this->fileSystem->basename($file->getFileUri()));

However, FileUploadResource and TemporaryJsonapiFileFieldUploader only change the filename property of a File entity if

  1. There is a security rename

Steps to reproduce

Proposed resolution

Update FileUploadResource and TemporaryJsonapiFileFieldUploader to match the behaviour of the core file field upload. See#2.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3402981

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

kim.pepper created an issue. See original summary.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

I think it makes sense to make the behaviour consistent here.

This comment on the name field in the file entity indicates the URI and filename should be consistent

BaseFieldDefinition::create('string')
      ->setLabel(t('Filename'))
      ->setDescription(t('Name of the file with no path components.'));

kim.pepper opened merge request !5486

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
commented
Status: Active » Needs review

Looks like the change is minor. Needed to update the REST test expectations.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Seems simple enough

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
commented
Title: Filename property is not updated when a file is renamed FileUploadResource » Filename property is not updated when a duplicate file is renamed In REST and JSON API file uploads
kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
commented
Issue summary: View changes
kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
commented
Issue summary: View changes
quietone’s picture

quietone commented
Issue summary: View changes
Status: Reviewed & tested by the community » Needs work

I'm triaging RTBC issues. I read the IS and the comments. Thanks for the links in the IS, it made this easy to review.

The proposed resolution read as if confirmation was still needed. Since that is in #2 I updated the proposed resolution..

I left a question in the MR so I am setting back to NW for a reply.

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
commented
Status: Needs work » Needs review
smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Feedback from @quietone appears to have been addressed.

  • larowlan committed 501e1ed2 on 11.x 
    Issue #3402981 by kim.pepper, larowlan, quietone: Filename property is...
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Reviewed & tested by the community » Fixed

Committed to 11.x

Added and published a change record for the behaviour change

larowlan closed merge request !5486

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Issue tags: +Bug Smash Initiative

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.