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

Remove agrovoc linked data authority #183

Merged
merged 1 commit into from Nov 19, 2018
Merged

Remove agrovoc linked data authority #183

merged 1 commit into from Nov 19, 2018

Conversation

elrayle
Copy link
Contributor

@elrayle elrayle commented Nov 19, 2018

The URL for the AgroVoc API has changed twice in the last year and a half. It is undesirable to have to update QA for the changing API requirements. The AgroVoc configuration will continue to be maintained at LD4P/linked_data_authorities/qa_agrovoc.

NOTE: At this writing, the configuration at LD4P Linked Data Authorities is broken. It will be fixed as soon as the AgroVoc technical problems are resolved.

The URL for the AgroVoc API has changed twice in the last year and a half.  It is undesirable to have to update QA for the changing API requirements.  The AgroVoc configuration will continue to be maintained at https://github.com/LD4P/linked_data_authorities/tree/master/qa_agrovoc.
@@ -9,5 +9,5 @@
# action '/reload/linked_data/authorities?auth_token=YOUR_AUTH_TOKEN_DEFINED_HERE' without
# requiring a restart of rails. By default, reloading through the browser is not allowed
# when the token is nil or blank. Change to any string to control who has access to reload.
# config.authorized_reload_token = YOUR_AUTH_TOKEN_DEFINED_HERE
# config.authorized_reload_token = 'YOUR_AUTH_TOKEN_DEFINED_HERE'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sneaking in a quick fix to make this a more accurate example since the token users change to should be a string. No code impact with this change.

expect(response).to be_successful
expect(response.content_type).to eq 'application/ld+json'
expect(JSON.parse(response.body).keys).to match_array ["@context", "@graph"]
end
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These tests were covered using the Agrovoc auth. They were moved to the oclc_fast tests to maintain coverage of that functionality.

@elrayle
Copy link
Contributor Author

elrayle commented Nov 19, 2018

NOTE: I confirmed that any functionality that was tested solely under tests related to agrovoc were moved to another authority to avoid a drop in test coverage.

@revgum revgum merged commit 7a7fed7 into master Nov 19, 2018
@revgum revgum deleted the remove_agrovoc branch November 19, 2018 19:06
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.

None yet

2 participants