add resource policy for download to match show

[jmilljr24]

Closes #1908

What is the goal of this PR and why is this important?

Fix resource download link for unauthenticated-users

How did you approach the change?

skip authentication check for download and add download policy

UI Testing Checklist

Anything else to add?

[jmilljr24]

@maebeale I think this is fine for a quick fix.

Question:
This highlighted a discrepency on the Resource show page. There are two places a user can click to download the asset. 1. the download button 2. the preview image link. The download button uses the download controller action. The preview link is a direct url_for(@file).

To some degree I think that both should follow the same pattern. Using the direct url makes sense because the show page is already gated to the correct policy on who can see (in turn who can download). The download button/action however taps into the the ahoy tracking which we don't have with the direct url.

I can create an issue for this, just wanted to see which direct you thought before I wrote it up. It is a low priority.

[maebeale]

i don't think show will be true for these hidden_from_search resources. i think true is better, or, we add show? || record.hidden_from_search?, or, update the show policy action.

[jmilljr24]

There is a mix up between the policy scope and the policies related to controller actions.

hidden_from_search is not a controller action, it is just the flag the search scope looks at to filter out results.

I'm confident show? is set up correctly. A resource that is hidden from search would still need the correct settings for the correct users to see it regardless of if it is hidden from search or not. In turn, if a user can see the show page, they should be able to download it as well.

[maebeale]

regular_user should be able to download it too. they may still refer to their ticket from time to time.

[jmilljr24]

This test is actually unneeded. hidden_from_search is only used on the scope for search results. There is no direct policy gated on it.

[maebeale]

@jmilljr24 i don't think the policies are right yet

[maebeale]

@jmilljr24 i'm fine w consolidating Resource download behavior, though i thought there were cases for both. def should add the ahoy tracking to direct if we keep it or pick it as the unified behavior. if we pick one i pick direct where you get the full preview.

[maebeale]

i don't think the policy is correct yet so surprised tests aren't catching it. i must be missing something.

[jmilljr24]

i pick direct where you get the full preview.

I'm not sure I know what you mean by this.

[jmilljr24]

i don't think the policy is correct yet so surprised tests aren't catching it. i must be missing something.

I tried to explain it on the individual comments but here is the big picture:

resources/1 hits the show action -> show? policy

def show?
    admin? || record.publicly_visible? || (authenticated? && record.published?)
  end

So if you are authorized to view the show page, you should be authorized to download. There is no reason to duplicate the show method so I made download? call the show? method. Realistically I could have use an alias but I know that action policy isn't always intuitive if you don't work with it much. The registration ticket routes never hit the resource show action or policy. So the user can see the registration version of the resource, the download button which was copied from there resource show still points to resource#download.

hidden_from_search is only used on the Resource index results.