Problem/Motivation

Button icons are lost after cron run because there icons are not tracked with cores file.usage service.

Proposed resolution

Maintain file usage information with CRUD hooks.

Remaining tasks

Test

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#18 track-button-file-usage-2537786-18.patch8.74 KBDevin Carlson
#18 track-button-file-usage-2537786-18-tests-only.patch2.19 KBDevin Carlson
#16 track-button-file-usage-2537786-16.patch8.73 KBDevin Carlson
#8 2537786-8.patch8.9 KBwebflo
#5 2537786-5.patch4.71 KBwebflo
#1 2537786.patch2.72 KBwebflo
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

webflo’s picture

webflo CreditAttribution: webflo at UEBERBIT GmbH commented
Issue summary: View changes
FileSize
2537786.patch2.72 KB
Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid as a volunteer commented

Hrm, are the files not getting set as a permanent file to begin with? That should have prevented this in the first place, but I agree that adding usage is good. I think this code might need to be in the Entity class itself possibly, and not in a hook.

Status: Needs review » Needs work

The last submitted patch, 1: 2537786.patch, failed testing.

webflo’s picture

webflo CreditAttribution: webflo at UEBERBIT GmbH commented

Files are temporary by default. FileFieldItemList::postSave maintains the file usage for file and image fields. I implemented it as hooks because thought it's not the concern of the entity to integrate in other subsystems.

webflo’s picture

webflo CreditAttribution: webflo at UEBERBIT GmbH commented
Status: Needs work » Needs review
FileSize
2537786-5.patch4.71 KB
webflo’s picture

webflo CreditAttribution: webflo at UEBERBIT GmbH commented 
+++ b/src/Tests/EmbedButtonAdminTest.php
@@ -99,4 +100,42 @@ class EmbedButtonAdminTest extends EntityEmbedTestBase {
+    \Drupal::configFactory()->getEditable('system.file')->set('temporary_maximum_age', 1)->save();

This line is not necessary. I remove it during the next reroll.

Status: Needs review » Needs work

The last submitted patch, 5: 2537786-5.patch, failed testing.

webflo’s picture

webflo CreditAttribution: webflo at UEBERBIT GmbH commented
Status: Needs work » Needs review
FileSize
2537786-8.patch8.9 KB

Status: Needs review » Needs work

The last submitted patch, 8: 2537786-8.patch, failed testing.

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid as a volunteer commented

Because the icon is an integral part of the config entity itself and not something optional, I don't think hooks seem like something useful for this.

I implemented it as hooks because thought it's not the concern of the entity to integrate in other subsystems.

If this were true then the file module would be implementing hooks scanning for file and image field references for tracking usage, instead of it being part of the file and image fields themselves.

webflo’s picture

webflo CreditAttribution: webflo at UEBERBIT GmbH commented

Latest patch is on GitHub https://github.com/drupal-media/entity_embed/pull/151

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid as a volunteer commented
Status: Needs work » Fixed

  • webflo committed 30e0b1a on 8.x-1.x 
    Issue #2537786: Button icons are deleted after cron

Status: Fixed » Closed (fixed)

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

Devin Carlson’s picture

Devin Carlson CreditAttribution: Devin Carlson commented
Version: 8.x-1.x-dev » 7.x-2.x-dev
Status: Closed (fixed) » Patch (to be ported)
Devin Carlson’s picture

Devin Carlson CreditAttribution: Devin Carlson commented
Status: Patch (to be ported) » Needs review
FileSize
track-button-file-usage-2537786-16.patch8.73 KB

A backport of #13.

Status: Needs review » Needs work

The last submitted patch, 16: track-button-file-usage-2537786-16.patch, failed testing.

Devin Carlson’s picture

Devin Carlson CreditAttribution: Devin Carlson commented
Status: Needs work » Needs review
FileSize
track-button-file-usage-2537786-18-tests-only.patch2.19 KB
track-button-file-usage-2537786-18.patch8.74 KB

The last submitted patch, 18: track-button-file-usage-2537786-18-tests-only.patch, failed testing.

Devin Carlson’s picture

Devin Carlson CreditAttribution: Devin Carlson commented
Status: Needs review » Fixed

Tested #18 and committed to Entity Embed 3.x, 2.x and 1.x.

  • Devin Carlson committed d410b1f on 7.x-3.x 
    Issue #2537786 by Devin Carlson: Fixed button icons are deleted after...

  • Devin Carlson committed 96a9360 on 7.x-2.x 
    Issue #2537786 by Devin Carlson: Fixed button icons are deleted after...

  • Devin Carlson committed 8e80fed on 7.x-1.x 
    Issue #2537786 by Devin Carlson: Fixed button icons are deleted after...

Status: Fixed » Closed (fixed)

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