Move file entity info, managed file, and file usage functionality into File module

[jenlampton]

Drupal.org issue: https://drupal.org/node/1468328

File module used to be an optional module, with the "required" portions living in System module. However File module has been required for all the D7 cycle. We should move the relevant file functionality out of System and into File.

---

PR by @Al-Rozhkov backdrop/backdrop#1829

[quicksketch]

This issue is related to #152, which created the File entity. To speed the Entity conversion, we left the File entity where it was in System module. Now that the conversion is complete, it'd be good to get it moved into File.

[jenlampton]

Also related #405

[jenlampton]

Also related #2375

[quicksketch]

This definitely isn't going to happen for 1.6.0. Moving to next release.

[klonos]

This would make the file.module a required one. Correct?

[Al-Rozhkov]

PR: backdrop/backdrop#1829

Please, review.

[klonos]

@Al-Rozhkov above my skillset for review, but what about my question above? Do you think that file becomes a required module now?

[Al-Rozhkov]

To be honest this is above my skills too. I just really want media library in Backdrop core and I try to learn as much as possible to make it happen. Backdrop is so much better than WordPress in all aspects except for file management. This is the last obstacle for me to use Backdrop in all my sites.

As far as I understand things File module needs to be required. Looks like some parts of System and User modules use File module.

I disabled File module and discover few things.

1. Manage files page is still there but views handler is missing.
2. "Insert image" functionality still exist in text editors (without Image and File modules).

While I was writing this comment I realized that my PR doesn't add required = TRUE to the File module. So back to "needs work".

[Al-Rozhkov]

The more I dive into it, the more I think that we can keep File module unrequired :)

I thought that user pictures dependent on Image module, but looks like it is not. There is simple if (module_exists('image')) check.

All file related API exist in core/includes/file.inc and File module is for file entities, file usage and file field functionality.

[quicksketch]

Excellent work @Al-Rozhkov! I can help fix the failing tests.

It seems like making File module required would be a pretty minor concern, but I'll check what's needed to just get tests passing.

[Al-Rozhkov]

Thanks a lot @quicksketch! It would be great. I know nothing about tests yet.

[jenlampton]

I also like the idea of having file module be required, actually. I think that's a non-issue either way. Good work everyone (especially @Al-Rozhkov) on moving this issue forward! :)

[quicksketch]

Hm, yeah on further examination here's the issue most tests are running into:

• Saving any type of content that contains filtered text.
• The filtering then parses for file IDs with _filter_record_file_usage().
• In the act of saving file usages, a new File object is created.

So making this change means that effectively filter.module becomes dependent on file.module.

Other places like block.module have bits of code like this:

  // Save each file as permanent, preventing it from being deleted. The same
  // process is used in Layout::save(), but that only applies to non-reusable
  // blocks (BlockText objects). If a block has been marked reusable, the files
  // are immediately marked as permanent.
  // File usages are not currently removed for custom blocks.
  // See https://github.com/backdrop/backdrop-issues/issues/2137.
  $fids = filter_parse_file_fids($block['body']['value']);
  $files = file_load_multiple($fids);
  foreach ($files as $fid => $file) {
    if ($file && $file->status !== FILE_STATUS_PERMANENT) {
      // This makes the file "self-referencing", so it will never be deleted.
      file_usage_add($file, 'file', 'file', $file->fid);
    }
  }

So again this is assuming the API for File "usages" is always available. From a backwards-compatibility standpoint, I think we need to make sure it remains that way.

[quicksketch]

@Al-Rozhkov Could you add the following line to file.info and push to the PR?

required = TRUE

[Gormartsen]

Just commit this to your repo, that a source for PR. PR will be synced and test run again. Regards, Gor Martsen @Gormartsen

…

On Apr 16, 2017, at 3:58 PM, Nate Haug ***@***.***> wrote: @Al-Rozhkov Could you add the following line to file.info and push to the PR? required = TRUE — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[Al-Rozhkov]

Thank you everyone! It is so exciting to learn git and Backdrop!

[quicksketch]

All tests are now passing and I can't find any issues in manual testing.

I've merged backdrop/backdrop#1829 into 1.x for 1.7.0. Great job @Al-Rozhkov!