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

controllers: adds $schema to payload #23

Closed
wants to merge 1 commit into from

Conversation

pamfilos
Copy link
Member

@pamfilos pamfilos commented Sep 1, 2016

Signed-off-by: Pamfilos Fokianos pamfilosf@gmail.com

Signed-off-by: Pamfilos Fokianos <pamfilosf@gmail.com>
@@ -185,6 +185,7 @@ function InvenioRecordsCtrl($scope, $rootScope, $q, $window, $location,
url: vm.invenioRecordsEndpoints[type],
method: (method || 'PUT').toUpperCase(),
data: {
'$schema': vm.invenioRecordsEndpoints.schema,
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be a part of _data (metadata)? @lnielsen @drjova

Copy link
Member Author

Choose a reason for hiding this comment

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

No.. For it to work now it should be on the same level as metadata, not inside

@lnielsen
Copy link
Member

lnielsen commented Sep 1, 2016

We need to refactor how the JSON payload is provided. E.g. adding $schema will not work for the Zenodo REST API. I think the best is that we can provide a callback which can "massage" the payload before sending it.

@drjova Any ideas for how best to solve it?

@lnielsen
Copy link
Member

lnielsen commented Sep 1, 2016

Additionally, since we're soon to release Zenodo, we probably need two branches (maint/master, master/next), so that we don't block each other.

@drjova
Copy link
Member

drjova commented Sep 2, 2016

Superseded by f0c65e2 you can now add additional data to the payload by using the existing extra-params attribute

<invenio-records
  extra-params='{"headers": {"foo": "bar"}, "data": {"$schema": "foofoo"}}'
>
</invenio-records>

With that fix you it'll work exactly as it is on invenio-search-js any Invenio app can have its own custom data, headers etc. :)

@lnielsen
Copy link
Member

lnielsen commented Sep 2, 2016

@drjova But I think the problem is as well that it's adding the {'metadata': ..} key…..which is needed on zenodo but not analysis preservation

@drjova
Copy link
Member

drjova commented Sep 2, 2016

@drjova
Copy link
Member

drjova commented Sep 5, 2016

superseded by #24

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.

4 participants