Skip to content
This repository has been archived by the owner on Nov 2, 2021. It is now read-only.

Core module: update textValueHasMapping #87

Merged
merged 21 commits into from
Nov 28, 2018

Conversation

flavens
Copy link
Collaborator

@flavens flavens commented Oct 3, 2018

textValueHasMapping is not a string anymore, but an object

"knora-api:textValueHasMapping" : {
    "@id" : "http://rdfh.ch/standoff/mappings/StandardMapping"
 }

Files to update:

  • projects/knora/core/src/lib/declarations/api/knora-constants.ts
  • projects/knora/core/src/lib/services/v2/convert-jsonld.ts
  • projects/knora/core/src/lib/test-data/resources/Testthing.json
  • projects/knora/core/src/lib/test-data/resources/SearchForHolzschnitt.json

@flavens flavens added the enhancement New feature or request label Oct 3, 2018
@flavens flavens self-assigned this Oct 3, 2018
@flavens
Copy link
Collaborator Author

flavens commented Oct 3, 2018

cf Knora PR#999

@flavens
Copy link
Collaborator Author

flavens commented Oct 8, 2018

review PR

@benjamingeer
Copy link
Contributor

@kilchenmann @tobiasschweizer Could somebody please review this PR?

Copy link
Contributor

@kilchenmann kilchenmann left a comment

Choose a reason for hiding this comment

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

We have to know, which Knora version is now required for these changes? Can someone add this information to the README of the module!?

@benjamingeer
Copy link
Contributor

It will be Knora 2.0.1 (not released yet).

@kilchenmann
Copy link
Contributor

kilchenmann commented Oct 19, 2018

great! thanks. I will add it to the README before merging

Copy link
Contributor

@kilchenmann kilchenmann left a comment

Choose a reason for hiding this comment

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

add knora version to the readme (or somewhere). We have to do that for now on...

@benjamingeer
Copy link
Contributor

I have just noticed another JSON-LD string that should be an object:

"knora-api:fileValueAsUrl" : "http://localhost:1024/knora/incunabula_0000004159.jp2/full/3625,5251/0/default.jpg"

should be

"knora-api:fileValueAsUrl" : {
  "@type" : "xsd:anyURI",
  "@value" : "http://localhost:1024/knora/incunabula_0000004159.jp2/full/3625,5251/0/default.jpg"
}

@benjamingeer
Copy link
Contributor

And another one: knora-api:stillImageFileValueHasIIIFBaseUrl.

@benjamingeer
Copy link
Contributor

@tobiasschweizer I think I've changed the code here to accept JSON objects in knora-api:fileValueAsUrl and knora-api:stillImageFileValueHasIIIFBaseUrl, as produced by dasch-swiss/dsp-api#1011. Could you please check whether this works?

@tobiasschweizer
Copy link
Contributor

tobiasschweizer commented Nov 21, 2018

Alternative way to filter out preview images from file values:

filterOutPreviewImages(fileValue: ReadStillImageFileValue) {
        const isPreview: boolean = fileValue.imageFilename.indexOf('.jpg') !== -1;
        return !isPreview;
    }

This method can be passed to filter when applied to a collection of file values:

const fileValues: ReadStillImageFileValue[] = ...
fileValues.filter(filterOutPreviewImages);

@tobiasschweizer
Copy link
Contributor

@benjamingeer I think I found a good solution to determine whether a file value is a preview or not by looking at the file extension (
a055a82)

I will now add more test for JSON-LD conversion.

@tobiasschweizer
Copy link
Contributor

@benjamingeer Ok, I added more test for StillImageRepresentation with image file values and TextRepresentation with text file values.

Determining whether an image is a preview or not is now handled without fileValueIsPreview. The assumption is that previews are JPEGS.

@tobiasschweizer
Copy link
Contributor

I think the ontology test data has to be adapted too

@tobiasschweizer
Copy link
Contributor

@kilchenmann added required Knora version in
71b8dd8

@tobiasschweizer tobiasschweizer merged commit aa993a5 into master Nov 28, 2018
@tobiasschweizer tobiasschweizer deleted the wip/textValueHasMapping-update branch November 28, 2018 14:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants