Related Issues

Subtask of #1551384: Port project_release to D7
This issue needs a project-module-specific file field formatter, but is being blocked by #1609382: Port release files with extra metadata to D7

Problem/Motivation

In D6, we had to do horrible hacks to make the 'project_release_download_base' setting work. In D7, we can now just use hook_file_url_alter(). I think we still need a release-specific setting for this, although while we're at it, we should just move that to a per-content-type setting instead of trying to do this site-wide. However, to "enforce" the setting, we no longer need theme_project_release_download_link(), we can just use the alter hook.

Thanks to davereid in IRC for pointing out this alter hook to me. ;)

Next Steps

  1. Write a field formatter (see https://drupal.org/node/1606656#comment-6317874 below)

Comments

Dave Reid’s picture

Yeah as long as however the file field is being output uses file_create_url() (which all instances of core should be already), then the hook should be a great fit for this.

dww’s picture

Ugh, except there's no field data passed to the hook. :( so how is project_release supposed to know if it's altering a release download link or not?

Wim Leers’s picture

Can't it know this based on what directory the download is in? Probably something like /files/projects/*?

dww’s picture

I thought of using a file subdir convention and triggering this functionality based on that, but it seemed like a fragile hack. What if something else was using the same subdir? What if we have so many release files on a site (tee hee, something d.o sized) that we wanted to shard the tarballs into N subdirs? Seems like the UI for configuring this (to specify which file subdir(s) to rewrite) would be a mess.

Surely there's a better way... Right? ;)

I guess I need to investigate where this alter hook is being invoked to see if it's even possible to provide more context. I suspect the problem is that not all files touched by this hook are even file fields. Sounds like it's used at a pretty low level, eg for CSS/JS to be included with a page request...

Dave Reid’s picture

I'm beginning to suspect that we may want a project module-specific field formatter for the release file fields that just does the output the way we need it to be output.

dww’s picture

Title: Port the 'project_release_download_base' setting to D7, make it per-node-type, and use hook_file_url_alter() » Port the 'project_release_download_base' setting to D7 and make it per-node-type

Yeah, seems like hook_file_url_alter() isn't going to work after all. :/

I suppose a custom file formatter that should always be used for release file fields makes sense. That's sorta like what we do in D6 already with our special theme function, only cleaner and more sane. ;)

drumm’s picture

Assigned: Unassigned » drumm

http://drupalcode.org/project/project.git/commit/ca69fd7 has the setting UI. The field formatter is remaining.

dww’s picture

http://drupalcode.org/project/project.git/commitdiff/ca69fd7 looks great, thanks. Just need that field formatter now. ;)

dww’s picture

Issue summary: View changes

thanking Dave Reid for the tip

Senpai’s picture

Status: Active » Postponed
Issue tags: +sprint 8

Postponed pending the outcome of #1609382: Port release files with extra metadata to D7 because we need files and stuff in order to, ya know, test a project-module-specific file field formatter.

Senpai’s picture

Issue summary: View changes

Adding a Next Steps section.

dww’s picture

Status: Postponed » Active

I don't think this needs to be blocked on #1609382. Someone can write the field formatter at any time (takes 1 minute to click a file field into existence), and we can use it when configuring the default release node type. IMHO, these are related (insofar as they involve release files) but basically independent issues.

drumm’s picture

Coming back to this now that we have feed URLs being displayed on release nodes. I don't hink a field formatter will work since the two places this shows are

  • The project_release_files view inserted into node rendering.
  • Calling field_view_value() for RSS.

Neither of these is a straightforward field rendering. hook_file_url_alter() could work, but it only knows about the URL, we would have to depend on the path.

Wim Leers’s picture

#11: correct, hook_file_url_alter()'s only information is the URL itself. The question is: is that acceptable?

drumm’s picture

I settled on

function project_release_field_attach_view_alter(&$output, $context) {
  if (isset($output['field_release_file'])) {
    foreach (element_children($output['field_release_file']) as $key) {
      if ($wrapper = file_stream_wrapper_get_instance_by_uri($output['field_release_file'][$key]['#file']->uri)) {
        list($scheme, $target) = explode('://', $wrapper->getUri(), 2);
        $path = $wrapper->getDirectoryPath() . '/' . str_replace('\\', '/', trim($target, '\/'));
      }
      $output['field_release_file'][$key]['#suffix'] = '!!';
    }
  }
}

This works for altering the URL everywhere. Now that the setting is per-node-type, we need to know the node type. $context is the field collection, so it isn't immediately known.

drumm’s picture

Status: Active » Fixed

Found FieldCollectionItemEntity->hostEntityBundle() for the node type and committed.

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

Anonymous’s picture

Issue summary: View changes

This issue only needs a project-module-specific file field formatter, but is being blocked by #1609382: Port release files with extra metadata to D7