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

BUG Keep folder Name and Title in sync on update #166

Merged
merged 2 commits into from
Oct 8, 2018

Conversation

lukereative
Copy link

@lukereative lukereative commented Oct 1, 2018

This was discovered while working to improve folder sorting, folders are sorted by Title but when you update their Name through the admin interface it would not update the Title and they would end up out of sync and quietly mess up alphabetical sorting.

Related but not dependant PR silverstripe/silverstripe-asset-admin#844

Copy link
Contributor

@robbieaverill robbieaverill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a behavioural change which might not be intended. Can we have a unit test for this please? That might justify the change a bit more

@robbieaverill
Copy link
Contributor

I'm kind of tempted to suggest that this is a feature, not a bug

@lukereative
Copy link
Author

lukereative commented Oct 4, 2018

It doesn't seem like a feature, it seems to be an inconsistency because we already do it similarly on 'setTitle'

public function setTitle($title)
{
$this->setField('Title', $title);
$this->setField('Name', $title);
return $this;
}

And it's causing an issue with asset admin, I'm going to add a unit test.

@robbieaverill
Copy link
Contributor

Nice one!

{
parent::onBeforeWrite();

$this->Title = $this->getField('Name');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So you can still manually change the Title field because this is in a beforeWrite right?

@robbieaverill
Copy link
Contributor

Nice branch name

Copy link
Contributor

@robbieaverill robbieaverill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code isn't immediately obvious why it works, but it does =D

Test case:

  • Upload a file "dog.jpg". Title is set to "dog"
  • Replace it with "cat.jpg". Title is set to "cat"
  • Change the title to "my pets", save
  • Replace file with "dogs.jpg". Title is not changed from "my pets"
  • Replace file with "cats.jpg". Title is not changed from "my pets"

👍

@robbieaverill robbieaverill merged commit 6d67489 into silverstripe:1 Oct 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants