Skip to content

Conversation

elpoelma
Copy link
Contributor

@elpoelma elpoelma commented Jun 3, 2025

Overview

This PR ensures that older-style literal nodes (which are not per se rdfa-aware) are more or less correctly parsed.
The example used to test this PR are older decision templates with non rdfa-aware titles.

connected issues and PRs:

GN-5607

How to test/reproduce

  • Start the test-app
  • Insert an older decision template, e.g.:
<div
    lang="nl-BE"
    data-say-document="true"
    data-literal-node="true"
>
    <div
        style="display: none"
        class="say-hidden"
        data-rdfa-container="true"
    ></div>
    <div data-content-container="true">
        <div
            class="say-editable say-block-rdfa"
            about="http://data.lblod.info/id/besluiten/000f9360-2fcc-11f0-ad19-fbd5cd20271e"
            data-say-id="30e2d215-a8b6-4bab-9d8b-aaab634589ea"
        >
            <div
                style="display: none"
                class="say-hidden"
                data-rdfa-container="true"
            ><span
                    about="http://data.lblod.info/id/besluiten/000f9360-2fcc-11f0-ad19-fbd5cd20271e"
                    property="http://www.w3.org/1999/02/22-rdf-syntax-ns#type"
                    resource="http://data.vlaanderen.be/ns/besluit#Besluit"
                ></span><span
                    about="http://data.lblod.info/id/besluiten/000f9360-2fcc-11f0-ad19-fbd5cd20271e"
                    property="http://www.w3.org/1999/02/22-rdf-syntax-ns#type"
                    resource="http://mu.semte.ch/vocabularies/ext/BesluitKlassiekeStijl"
                ></span><span
                    about="http://data.lblod.info/id/besluiten/000f9360-2fcc-11f0-ad19-fbd5cd20271e"
                    property="http://www.w3.org/1999/02/22-rdf-syntax-ns#type"
                    resource="https://data.vlaanderen.be/id/concept/BesluitType/e96ec8af-6480-4b32-876a-fefe5f0a3793"
                ></span><span
                    property="http://data.europa.eu/eli/ontology#title"
                    content="Decision title"
                    datatype="http://www.w3.org/2001/XMLSchema#string"
                ></span><span
                    property="http://data.europa.eu/eli/ontology#title"
                    content="Decision title edit"
                    datatype="http://www.w3.org/2001/XMLSchema#string"
                ></span><span
                    rev="http://www.w3.org/ns/prov#generated"
                    resource="http://example.org#"
                ></span></div>
            <div data-content-container="true">
                <h4
                    data-indentation-level="0"
                    style=""
                    level="4"
                    indentationlevel="0"
                    alignment="left"
                    property="http://data.europa.eu/eli/ontology#title"
                    about="http://data.lblod.info/id/besluiten/000f9360-2fcc-11f0-ad19-fbd5cd20271e"
                    datatype="http://www.w3.org/2001/XMLSchema#string"
                    lang=""
                    class="say-heading"
                >Decision title edit</h4>
            </div>
        </div>
    </div>
</div>
  • Notice that the title included in this sample decision is not rdfa-aware (rdfa-attrs are define on the header tag itself, there is not data-literal-node attribute...)
  • Insert this html in the editor
  • The relationship to the title is parsed as a relationship to a literal node
  • As the heading node itself is not rdfa-aware, it does not have backlinks defined
  • Changing the content of the heading node works and results in the correct changes in the RDF output

Challenges/uncertainties

This is not a perfect solution, as heading nodes are (no longer) rdfa-aware.
An alternative solution would be to convert these type of headings to a div node (block_rdfa) with a plain heading node inside.
AFAIK, resource nodes may not have a datatype attribute, while literal nodes always have one.

Checks PR readiness

  • UI: works on smaller screen sizes
  • UI: feedback for any loading/error states
  • Check cancel/go-back flows
  • Check database state correct when deleting/updating (especially regarding relationships)
  • changelog
  • npm lint
  • no new deprecations

@elpoelma elpoelma self-assigned this Jun 3, 2025
@elpoelma elpoelma added the bug Something isn't working label Jun 3, 2025
@elpoelma elpoelma force-pushed the GN-5607-fix-old-template-literal-node-parsing branch from f7611f8 to 6d10919 Compare June 3, 2025 12:47
@elpoelma elpoelma marked this pull request as draft June 3, 2025 13:06
@elpoelma
Copy link
Contributor Author

elpoelma commented Jun 3, 2025

It seems some issues popped-up with this change in the external-triple tests, will investigate this further...

@elpoelma elpoelma force-pushed the GN-5607-fix-old-template-literal-node-parsing branch 2 times, most recently from 91db7b4 to 8b9aad3 Compare June 12, 2025 12:54
@elpoelma elpoelma changed the base branch from master to GN-5604-fix-literal-node-rels June 12, 2025 13:48
@elpoelma
Copy link
Contributor Author

It seems some issues popped-up with this change in the external-triple tests, will investigate this further...

I've added a quick/hacky solution for the external triples. If this PR looks good to you, I'll refactor it in a more robust solution.

@elpoelma elpoelma marked this pull request as ready for review June 12, 2025 13:54
Copy link
Contributor

@piemonkey piemonkey left a comment

Choose a reason for hiding this comment

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

Looks good to me (after removing the duplicate title in the given example). I'm not sure of the best way to tell what is a resource, but I'm confident that they shouldn't have datatypes.

@abeforgit
Copy link
Member

Hmm I don't think literal nodes always have a datatype
If they have a language, their datatype is automatically set to LangString. That being said, I think the check is fine in this case

I'm curious about the "alternative" solution of wrapping it in a literal node, cause that's I think the ideal result. I'm assuming it's more of an additional thing right, since we'd still need the parser changes in this pr to make that possible?

@elpoelma
Copy link
Contributor Author

Hmm I don't think literal nodes always have a datatype If they have a language, their datatype is automatically set to LangString. That being said, I think the check is fine in this case

I'm curious about the "alternative" solution of wrapping it in a literal node, cause that's I think the ideal result. I'm assuming it's more of an additional thing right, since we'd still need the parser changes in this pr to make that possible?

I indeed think not all literal nodes have a datatype, but all nodes with a datatype should be literal nodes right (with the exception of external triples).

On the wrapping solution: I agree that this would be a nice solution, but I think that this parser change is indeed still necessary. The wrapping is something we'd do on the prosemirror level. I can look into it in a seperate PR maybe?

@elpoelma elpoelma force-pushed the GN-5604-fix-literal-node-rels branch from 57063f5 to a803ce2 Compare June 13, 2025 09:23
@elpoelma elpoelma force-pushed the GN-5607-fix-old-template-literal-node-parsing branch from 03dd619 to 4a02d11 Compare June 13, 2025 09:24
@elpoelma
Copy link
Contributor Author

@abeforgit I have included an example of the wrapping-strategy in #1329

@elpoelma elpoelma force-pushed the GN-5604-fix-literal-node-rels branch from a803ce2 to 329a9d1 Compare June 16, 2025 14:42
@elpoelma elpoelma force-pushed the GN-5607-fix-old-template-literal-node-parsing branch from a086289 to ab31283 Compare June 16, 2025 14:42
@abeforgit abeforgit merged commit 3de00c8 into GN-5604-fix-literal-node-rels Jun 18, 2025
5 checks passed
@abeforgit abeforgit deleted the GN-5607-fix-old-template-literal-node-parsing branch June 18, 2025 07:36
@abeforgit abeforgit mentioned this pull request Sep 1, 2025
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants