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

Add configuration form to remove ?_format=jsonld from @ids #33

Merged
merged 5 commits into from
May 21, 2019

Conversation

dannylamb
Copy link
Contributor

@dannylamb dannylamb commented May 15, 2019

GitHub Issue: Islandora/documentation#887

Supersedes #32

What does this Pull Request do?

This is the same code as #32 but on the Foundation's repo so that it can be tested with claw-playbook.

Makes a configuration choice of whether the URIs output by the Drupal normalizer contain the _format=jsonld parameter or not.

I also added a check in the getEntityUrl() for file types, they don't have a canonical link template but for now url() generates a good url.

What's new?

  • Added a setting form with a checkbox.

  • Does this change require documentation to be updated? included

  • Does this change add any new dependencies? no

  • Does this change require any other modifications to be made to the repository
    (ie. Regeneration activity, etc.)? no

  • Could this change impact execution of existing code? yes, toggling the checkbox could affect how items are serialized out to Fedora/Milliner/Triplestores/Solr.

How should this be tested?

  1. Pull down the PR
  2. Create a Repository Item (say http://localhost:8000/node/3)
  3. View the Json-ld ('http://localhost:8000/node/3?_format=jsonld`)
    • Note all the @ids have ?_format=jsonld on the end
  4. Goto http://localhost:8000/admin/config/search/jsonld
  5. Check the checkbox and Save the configuration.
  6. Reload the above Json-ld
    • Note the @ids no longer have ?_format=jsonld on the end.

Interested parties

@Islandora-CLAW/committers

@whikloj
Copy link
Member

whikloj commented May 16, 2019

@dannylamb You are missing the remove_jsonld_format from the install config schema.

@dannylamb
Copy link
Contributor Author

But it is there... I must be missing something subtle here.

@whikloj
Copy link
Member

whikloj commented May 17, 2019

@dannylamb do the tests pass if you run them locally?

@dannylamb
Copy link
Contributor Author

Nope, they don't run locally. I added the schema file as a precaution because I know it's finicky about this sort of stuff 😢 I just can't figure out what I'm doing wrong.

They do run just fine if I give it a little

  protected static $configSchemaCheckerExclusions = [
    'jsonld.settings',
  ];

but that feels like cheating.

@whikloj
Copy link
Member

whikloj commented May 17, 2019

@dannylamb I'll try to pull this into my claw-playbook and see if I can help

@dannylamb
Copy link
Contributor Author

@whikloj pfft.... it's boolean and not bool. wow.

@dannylamb
Copy link
Contributor Author

@dannylamb
Copy link
Contributor Author

Updating installer accordingly...

@dannylamb
Copy link
Contributor Author

aaaaaand done. Now we're properly using boolean when drush cseting as well.

@dannylamb
Copy link
Contributor Author

FYI https://www.drupal.org/project/config_inspector FTW

Copy link
Contributor

@seth-shaw-unlv seth-shaw-unlv left a comment

Choose a reason for hiding this comment

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

:shipit:

Anything need doing before I click merge?

@whikloj
Copy link
Member

whikloj commented May 21, 2019 via email

@seth-shaw-unlv seth-shaw-unlv merged commit faa8301 into 8.x-1.x May 21, 2019
@dannylamb dannylamb deleted the issue-887 branch May 30, 2019 15:48
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.

3 participants