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

Islandora tokens 1171 pt2 #845

Merged
merged 4 commits into from
Jul 27, 2021

Conversation

wgilling
Copy link

GitHub Issue: (Islandora/documentation#1171)

  • Other Relevant Links (Google Groups discussion, related pull requests,
    Release pull requests, etc.)

What does this Pull Request do?

This should complete the redistribution of all the tokens that were previously in the two distinct "islandora_tokens" modules (by @kayakr and @wgilling). Since this branch was once merged to islandora already, this new PR is to address the last change to islandora module to provide the pdf_url token. After this is merged, the full set of tokens would be available as part of islandora or the controlled_access_terms module (separate code repo and PR for that).

This PR moves all tokens that needed Media or File functionality into the islandora module.

What's new?

There should be no side-effects. If the instance of controlled_access_terms is not up to date, this PR may cause a conflict as to which module provides the functionality for this pdf_url token.

  • Added pdf_url token to islandora/islandora.tokens.inc
  • The pdf_url token has already been removed from the controlled_access_terms/controlled_access_terms.tokens.inc
  • Does this change require documentation to be updated? I didn't think there was any documentation for this yet.
  • Does this change add any new dependencies? nope
  • Does this change require any other modifications to be made to the repository
    (i.e. Regeneration activity, etc.)? clear the cache after pulling down the code
  • Could this change impact execution of existing code? it better not -- or else

How should this be tested?

  1. configure at least one of the Google Scholar tokens to use the pdf_url token
  2. navigate to several test items that have a PDF service file to see whether or not the token has been populated correctly in the ...

A description of what steps someone could take to:

  • Reproduce the problem you are fixing (if applicable) - does not apply
  • Test that the Pull Request does what is intended. - ensure that the PDF service file's fedora url is being set for the metatag seleted in step 1 of "How should this be tested?" above.

Interested parties

@dannylamb, @Islandora/8-x-committers

@wgilling wgilling force-pushed the islandora_tokens_1171_pt2 branch from 0038032 to d9af78f Compare July 22, 2021 22:30
if (!is_null($fid_value)) {
$file = File::load($fid_value);
if ($file) {
$url = $file->getFileUri();

Choose a reason for hiding this comment

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

This returns files with a protocol of public:// or fedora://, which can't be de-referenced from the outside. There's a function is IslanodraUtils you can use to get the download url of the file.

$url = $islandora_utils->getDownloadUrl($file); should do the trick.

Choose a reason for hiding this comment

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

Confirmed, it definitely does the trick. Make that change and this goes in 🚀 🙇‍♂️

Copy link
Author

Choose a reason for hiding this comment

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

getDownloadUrl replaced the previous code

Copy link

@dannylamb dannylamb left a comment

Choose a reason for hiding this comment

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

Awesome. Thanks @wgilling 🙇‍♂️

@dannylamb dannylamb merged commit 4a20d4b into Islandora:8.x-1.x Jul 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants