Skip to content

Add a removed records index#429

Merged
radudoros merged 9 commits into
developmentfrom
issue_396_removed_records_index
Jul 14, 2026
Merged

Add a removed records index#429
radudoros merged 9 commits into
developmentfrom
issue_396_removed_records_index

Conversation

@radudoros

Copy link
Copy Markdown

Add a separate removed records index that allows delivering partial information of deleted records. Addresses #396

@radudoros
radudoros requested a review from twagoo July 8, 2026 14:17
@radudoros radudoros self-assigned this Jul 8, 2026
@clarin-bot

clarin-bot commented Jul 8, 2026

Copy link
Copy Markdown

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
🔚 Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@twagoo twagoo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It looks good. I wasn't able to easily test it locally, as this requires records that are more than a day old (if maxDaysInSolr==0 then nothing is purged, as this would just delete everything, so the minimal is 1). However, I did notice that the importer 'hangs' in the end, the process doesn't terminate. If you can reproduce this, can you have a look at this? If not, we need to see if there are any specific conditions that may cause this.

Comment thread vlo-commons/src/main/java/eu/clarin/cmdi/vlo/FieldKey.java Outdated
@twagoo

twagoo commented Jul 9, 2026

Copy link
Copy Markdown
Member

A couple of small suggestions regarding the tombstone page (example screenshot below for reference)

  • I think it should be clearer at first sight that the record was not found. Perhaps the "visual title" of the page could be "Record not found" or "Record no longer available" instead of the value of the name field. The record's name could be displayed below that, I would think slightly smaller.
  • I think it would be more correct to write "This record was removed from the VLO on [date]"
  • Instead of " The information below (...) may help you find it elsewhere" I would write (".. may help you find the resource(s) that your are looking for").
  • If there is a landing page among the links, we could show it more prominently, yet in a way that doesn't imply that it is necessarily still available (e.g. "The delete record contained a reference to the following landing page: [url]")
image

@twagoo

twagoo commented Jul 9, 2026

Copy link
Copy Markdown
Member

If I'm not mistaken, the deleteAllFirst config option and deleteFirst option for dataroots do not cause records to be transferred to the morgue. We need to discuss whether this would be desirable.

@twagoo

twagoo commented Jul 10, 2026

Copy link
Copy Markdown
Member

And I believed that we agreed in the end about the 404 status code for the morgue record page. I believe this still needs to be implemented?

I was thinking of perhaps splitting this up, but this may need further discussion:

  • A 'not found' page with 404 status if the record is requested at the original URL /record/{id}
  • A tombstone page, either at a different URL or a variant of /record/{id} with a query parameter that has a regular 200 page

Some notes on that:

  • The former (not found page) could lead to the latter (tombstone) in the client, either manually (a clickable link) or automatically (e.g. using meta refresh)
  • If we want to link to morgue records e.g. from the search results, we point directly to the tombstone
  • The tombstone should point to the active record in case that has reappeared

@radudoros

Copy link
Copy Markdown
Author

I should have addressed all your comments - kindly help me with a rereview @twagoo

@twagoo twagoo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Some wicket things to look at, otherwise all looks good to me!

Comment thread vlo-web-app/src/main/java/eu/clarin/cmdi/vlo/wicket/pages/RecordPage.java Outdated
Comment thread vlo-web-app/src/main/java/eu/clarin/cmdi/vlo/wicket/pages/RecordPage.java Outdated
@radudoros
radudoros force-pushed the issue_396_removed_records_index branch from cc55b56 to a06f577 Compare July 14, 2026 13:50
@radudoros
radudoros merged commit e8c6c7d into development Jul 14, 2026
2 of 3 checks passed
@radudoros
radudoros deleted the issue_396_removed_records_index branch July 14, 2026 15:13
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.

3 participants