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

Lod #358

Merged
merged 127 commits into from
Dec 18, 2024
Merged

Lod #358

merged 127 commits into from
Dec 18, 2024

Conversation

MartinHinz
Copy link
Collaborator

Summary of Changes: LodLink Integration and Improvements

Based mostly on the work of @joeroe, I only extended the implementation to support multiple Linked Open Data (LOD) sources within a single database table. While the current implementation focuses on Wikidata, it is designed to easily incorporate additional sources in the future.

Key Concepts

With this new approach:

  • Every visit to a site automatically creates a LodLink database entry with a status of "pending", storing any potential matches.
  • Pending matches are visible to users but flagged with a warning to indicate they are unapproved.
  • In the LOD curation backend, curators can scope entries to list sites requiring manual approval, enabling straightforward curation workflows.
  • This design eliminates the need for a bulk database-wide check, relying instead on user interaction to flag potential links dynamically.

Enhancements:

  • Introduced a flexible LodLink model to support multiple linked open data (LOD) sources, not just Wikidata.
  • Implemented a polymorphic association (lod_linkable) for seamless integration with different models like Site.
  • Added the source and status attributes to the LodLink model:
  • source: Identifies the LOD source (e.g., “Wikidata”).
  • status: Tracks the approval state of the link (pending, approved, rejected).

Functional Updates:

  • Replaced the wikidata_link method with a generalized lod_links association to support various LOD sources.
  • Added scope pending_wikidata_link to filter objects with pending Wikidata links.
  • Refactored the with_potential_wikidata_match? method to work with the new LodLink model, specifically checking status == "pending" for Wikidata links.

Forms and Views:

  • Updated forms to handle the LodLink approval process:
  • Added support for approving LodLink records directly from the Site views.
  • Ensured that users with appropriate permissions can approve or reject LodLink suggestions.

Database Changes:

  • Added a status column to the LodLink table to track the approval state of each link.
  • Implemented a data migration strategy to migrate existing Wikidata links into the new LodLink structure.

Removed Caching:

  • Replaced cache-based potential match storage with database persistence for better tracking and auditing.

Follow-Up Work

  • Add support for additional LOD sources and implement appropriate fetching/parsing logic for each.
  • Enhance UI for managing and reviewing LodLink records across different models.
  • Create a meaningful routine for checking for necessary updates

joeroe and others added 30 commits December 20, 2023 10:10
Instead of turbo streams. Fixing a bug where the stream could be
broadcast before the user loads the page and subscribes to the stream.
Instead of turbo streams. Fixing a bug where the stream could be
broadcast before the user loads the page and subscribes to the stream.
@MartinHinz
Copy link
Collaborator Author

MartinHinz commented Dec 17, 2024

  • After creating a WikidataLink manually via the site show view, navigating to http://127.0.0.1:3000/lods/sites raises an error because the data attribute of that link is nil

Can you retest, please?

  • "LOD" is a bit opaque, both in the code and (especially) the UI. Could we come up with a more descriptive name: DataLink? ExternalLink?

Both I do not like very much either. Best would be probably ExternalDataLink, but this is already awful long. How do you like LinkedResource? But I would postpone this (not to long, since it gets more complicated to solve later on, when this is referred more in the code)

  • "MISSING_WIKIDATA_LINK" would be more consistent with the issues UI than "NEED_WIKIDATA_LINK"

done in 0d93a8d

  • For consistency with other views _lod_link should not be a card, just a plain <section>

done in ce4a41c

  • When I create a link manually (from the site show view), it shows as pending:

    • This results in a UI hint saying "Wikidata link inferred from site name", which is technically not correct
    • It would make more sense to me to skip straight to approved if manually added, either for all users or for admins

I added a Checkbox with which you can approve that link in 2b162e9

  • Perhaps in the future it would make sense to (optionally?) refine the match by geography and/or the entity type in Wikidata

Totally agree!

@MartinHinz
Copy link
Collaborator Author

Removing simple-icons from package.json causes the CI to fail, like this:

$ esbuild app/javascript/*.* --bundle --sourcemap --outdir=app/assets/builds --public-path=/assets --loader:.png=file --loader:.woff=file --loader:.woff2=file
Error: R] Could not resolve "simple-icons"

    app/javascript/application.js:16:60:
      16 │ ... siWikipedia, siWikimediacommons, siWikidata } from 'simple-icons'
         ╵                                                        ~~~~~~~~~~~~~~

  You can mark the path "simple-icons" as external to exclude it from the bundle, which will remove this error and leave the unresolved path in the bundle.

1 error
node:child_process:921
    throw err;
    ^

Error: Command failed: /home/runner/work/xronos.rails/xronos.rails/node_modules/@esbuild/linux-x64/bin/esbuild app/javascript/application.js --bundle --sourcemap --outdir=app/assets/builds --public-path=/assets --loader:.png=file --loader:.woff=file --loader:.woff2=file
    at genericNodeError (node:internal/errors:983:15)
    at wrappedFn (node:internal/errors:537:14)
    at checkExecSyncError (node:child_process:882:11)
    at Object.execFileSync (node:child_process:918:15)
    at Object.<anonymous> (/home/runner/work/xronos.rails/xronos.rails/node_modules/esbuild/bin/esbuild:221:28)
    at Module._compile (node:internal/modules/cjs/loader:1565:14)
    at Object..js (node:internal/modules/cjs/loader:1708:10)
    at Module.load (node:internal/modules/cjs/loader:1318:32)
    at Function._load (node:internal/modules/cjs/loader:1128:12)
    at TracingChannel.traceSync (node:diagnostics_channel:322:14) {
  status: 1,
  signal: null,
  output: [ null, null, null ],
  pid: 6377,
  stdout: null,
  stderr: null
}

How do you feel about getting it back in? Other suggestions?

@MartinHinz MartinHinz requested a review from joeroe December 17, 2024 14:32
@joeroe
Copy link
Contributor

joeroe commented Dec 17, 2024

Can you retest, please?

Works great now!

Both I do not like very much either. Best would be probably ExternalDataLink, but this is already awful long. How do you like LinkedResource? But I would postpone this (not to long, since it gets more complicated to solve later on, when this is referred more in the code)

LinkedResource makes sense to me. I agree it is not worth holding up a merge though - #372

I added a Checkbox with which you can approve that link in 2b162e9

This works, though I do not quite understand in which circumstances a user would want to add a link that is not approved?

Perhaps in the future it would make sense to (optionally?) refine the match by geography and/or the entity type in Wikidata

#373

Removing simple-icons from package.json causes the CI to fail, like this:

We can just remove it. I think it's left over from the old import pipeline – my fault, sorry. CI should now be passing.

Thanks for updating the changelog too. I made a small change there. The .9000 dev version convention is carried over from tidyverse versioning and probably wasn't obvious, sorry. The idea is that <current version>.9000 holds all unreleased changes, until the next release, when it will be bumped to the next major/minor/patch version as appropriate. Then we create a new .9000 heading and repeat.

@MartinHinz
Copy link
Collaborator Author

Thank you for clarifying this! On that note, I’ll open an issue (https://github.com/xronos-ch/orga/issues/113) to discuss the release schedule, as I believe we’re approaching a version that’s ready for release.

@MartinHinz MartinHinz merged commit 4dd9832 into master Dec 18, 2024
4 checks passed
@MartinHinz MartinHinz deleted the lod branch December 18, 2024 08:37
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.

2 participants