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 mapping content identifier to annotations #173

Closed
stefandesu opened this issue May 4, 2022 · 9 comments
Closed

Add mapping content identifier to annotations #173

stefandesu opened this issue May 4, 2022 · 9 comments
Labels
question Further information is requested

Comments

@stefandesu
Copy link
Member

stefandesu commented May 4, 2022

Let's assume annotations are only used to annotate mappings that live inside the same jskos-server instance.

If the target mapping of an annotation changes in certain ways, i.e. its members or its type is changed, an annotation with motivation assessing (with bodyValue either +1 or -1) should be marked as invalidated or potentially even deleted because it's not relevant anymore.

I personally would not delete an annotation automatically since it belongs to a different user and also the creator of the mapping could simply change a mapping, then change it back, abusing the system. I think the best way would be to simply save a mapping's urn:jskos:mapping:content: and/or urn:jskos:mapping:members: identifiers which can then be compared to the current mapping's identifier to see whether the annotation refers to an old version of that mapping. If we do it this way, then jskos-server doesn't even need to be aware of this mechanism at all. (Except when we implement gbv/cocoda#605 which consists of calculating the total "rating" of a mapping.)

The Web Annotation Data Model is pretty flexible, so I think it would be possible to add additional data to an annotation. Also we could via npm run upgrade add the information to all existing annotations (assuming that the associated mappings have not changed since the annotation was created).

What do you think, @nichtich?

(This came up in gbv/cocoda#667.)

@stefandesu stefandesu added the question Further information is requested label May 4, 2022
@nichtich
Copy link
Member

nichtich commented May 5, 2022

I would compare annotations to comments and edit suggestions in collaborative software such as Google Docs. If other people comment my text, I can close the comment and the comment disappears when I delete the commented section of text. Trying to prevent "abuse" should not be a technical goal anyway.

On the other hand urn:jskos:mapping:content: is all we need, so adding this identifier to all mappings is probably the way to go.

Additional thought: Annotating a mapping might imply same judgement of other "duplicate" mappings with same urn:jskos:mapping:content:. But that's another issue.

@stefandesu
Copy link
Member Author

stefandesu commented May 5, 2022

The thing is: I would add the urn:jskos:mapping:content: identifier on top of the URI because the URI is the only way we can correctly identify a specific mapping. The question is how we can save this identifier while keeping backwards compatibility and adhering to the Web Annotation Data Model? Maybe we could misuse the canonical field and save it there? It has a somewhat similar meaning. Otherwise we could also just add a custom field like we do with JSKOS all the time. 🙈

@nichtich
Copy link
Member

nichtich commented May 5, 2022

canonical looks good but unforunately it's intended for the canonical URI of the annotation while we need a URI of the mapping. Changing target to urn:jskos:mapping:content: identifier would make sense to me but likely requires several changes in Cocoda as well, no?

@stefandesu
Copy link
Member Author

canonical looks good but unforunately it's intended for the canonical URI of the annotation while we need a URI of the mapping. Changing target to urn:jskos:mapping:content: identifier would make sense to me but likely requires several changes in Cocoda as well, no?

Yeah, it would require changes in Cocoda, changes in jskos-server, breaks backwards compatibility, and does not make sense in my opinion. I mentioned canonical not because it is the right place for something like this, I know it's not intended to be used for that, that's why I wrote "misuse". Maybe it would be better to just add a custom property...

@nichtich
Copy link
Member

nichtich commented May 5, 2022

Reading the Web Annotation Data Model I found how this feature (making sure the identity of annotated target has not been changed) is intended to be solved:

change

{
  "target": "https://coli-conc.gbv.de/api/mappings/f8eff4e2-a6df-4d2c-8382-523072c59af7",
}

to

{
  "target": {
    "id": "https://coli-conc.gbv.de/api/mappings/981a003d-ae64-46b9-9571-04cbbe78c7fd",
    "state": { 
      "id": "urn:jskos:mapping:content:3df6513d6c645eefafc93e62f6e2f221c6624e1b"
    }
  }
}

but as you said, the change might not be worth the trouble, we could just do

{
  "target": "https://coli-conc.gbv.de/api/mappings/981a003d-ae64-46b9-9571-04cbbe78c7fd",
  "_state": "urn:jskos:mapping:content:3df6513d6c645eefafc93e62f6e2f221c6624e1b"
}

@stefandesu
Copy link
Member Author

stefandesu commented May 5, 2022

Oh, I missed this! Well, I wouldn't mind either option, but actually doing it how the spec is intended sounds right to me. Changing the format of target doesn't break backwards compatibility for Cocoda at least as long as the API requests still work the same way.

One more question: Should Cocoda be responsible for adding that information or should jskos-server do it? I'm actually not sure about this, but so far, jskos-server doesn't actually mess at all with the annotations besides adding a URI in field id and dealing with created and modified.

Edit: It would probably make sense for jskos-server to add that information if the target is part of the same instance. Since it is only additional information, I think it would be fine to add it if it's not already included in the payload.

stefandesu added a commit that referenced this issue May 6, 2022
- Adds new index.
- Adds mapping content identifier if possible.
@stefandesu
Copy link
Member Author

Suggested implementation in branch annotation-state.

@stefandesu
Copy link
Member Author

stefandesu commented May 6, 2022

I found out (unfortunately after almost having finished the implementation) that jskos-server needs to have a consistent format for the target property of annotations. So we can't have both strings and objects (which is what I had previously assumed). There are two solutions:

  1. Make sure all annotations use target.id instead of a string as target, via adjustments to the service methods and an upgrade script that has to be called after an upgrade. This is how it's currently implemented in the branch.
  2. Not use the official model, but rather your last suggestion (maybe STATE instead of _state because jskos-server removes all properties starting with an underscore). This would make things simpler and things wouldn't break if npm run upgrade wasn't called after an upgrade, but our annotation format would not be standard Web Annotation Data Model anymore.

@nichtich what do you think? My guess would be that you'd vote for option 2, but maybe I'm wrong about that. 😅

Edit: Note that both options work fine from a validation standpoint which is nice. target.id is already supported by our JSON Schema, no changes necessary, and additional properties also seem to be allowed.

stefandesu added a commit that referenced this issue May 10, 2022
Change annotation format and add mapping state if possible (#173)
@stefandesu
Copy link
Member Author

This is now part of v1.4.5 and can be taken into account when implementing gbv/cocoda#605 and gbv/cocoda#667.

@stefandesu stefandesu changed the title Invalidate or delete annotation when target changed Add mapping content identifier to annotations May 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants