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

[DL-6006] Add the DecisionArticles validator #12

Open
wants to merge 1 commit into
base: chore/dependency-maintenance
Choose a base branch
from

Conversation

Windvis
Copy link

@Windvis Windvis commented Jun 12, 2024

This ensures we use the same validations as the frontend. We simply copy paste the implementation for now since it is unlikely to change much once it is stable and it allows us to do extra things in the backend version if needed.

Related PRs:

Builds on #11 so that needs to be merged first.

This ensures we use the same validations as the frontend. We simply copy paste the implementation for now since it is unlikely to change much once it is stable and it allows us to do extra things in the backend version if needed.

Related code: lblod/frontend-loket#391
formGraph,
);

return typeof literal !== 'undefined' && literal !== null && Literal.toJS(literal);
Copy link
Author

@Windvis Windvis Jun 12, 2024

Choose a reason for hiding this comment

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

It seems that the store in this service is not a "ForkingStore" instance, but a regular rdflib one. The rdflib version seems to return null instead of undefined if there is no data in the graph.

We probably should:

  • also return null in the forking store version
  • also use a forking store in this service, instead of the rdflib version to make the behaviour between frontend and backend (or other services) more consistent

But I think that's out of scope for this feature since the workaround is simple enough.

Copy link
Author

Choose a reason for hiding this comment

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

I also removed some imports that weren't being used anymore.

@Windvis Windvis changed the title Add the DecisionArticles validator [DL-6006] Add the DecisionArticles validator Nov 4, 2024
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.

1 participant