Skip to content

The one with the id's #294

Open
thehabes wants to merge 16 commits intomainfrom
delete-id-negotiation-hotifx
Open

The one with the id's #294
thehabes wants to merge 16 commits intomainfrom
delete-id-negotiation-hotifx

Conversation

@thehabes
Copy link
Member

@thehabes thehabes commented Nov 4, 2025

From a high level, discovered while trying to delete glosses. Some annotations failed the '@id' check and caused the loop to stop, so the Gloss entity itself was never deleted. Which Annotations were deleted were random.

Discovered that id negotiating is screwing us up at the Annotation level. Good lord. Everything id negotiates now.

@thehabes thehabes marked this pull request as ready for review November 4, 2025 23:39
@thehabes thehabes requested a review from cubap as a code owner November 4, 2025 23:39
@thehabes thehabes requested a review from Copilot November 4, 2025 23:39

This comment was marked as duplicate.

thehabes and others added 4 commits November 4, 2025 17:48
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@thehabes thehabes requested a review from Copilot November 4, 2025 23:51

This comment was marked as duplicate.

thehabes and others added 5 commits November 4, 2025 18:00
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@thehabes thehabes requested a review from Copilot November 5, 2025 00:07
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Member

@cubap cubap left a comment

Choose a reason for hiding this comment

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

There are a few things to take care of with a constant you are changing in some cases. I can't help but think this would be simpler if you did the http/s processing just when you loaded these things, but this cannon seems to hit everything.


li.setAttribute("data-expanded", "true")
cachedFilterableEntities.set(obj["@id"].replace(/^https?:/, 'https:'), obj)
const negotiatedId = obj["@id"] ?? obj.id
Copy link
Member

Choose a reason for hiding this comment

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

@thehabes This is a good comment.

const containingListElem = elem.closest("deer-view")
// Be careful. The publish list stores items via http://, but everything else is https://. Beware the false mismatch.
const glossID = obj["@id"].replace(/^https?:/, 'https:')
const negotiatedId = obj["@id"] ?? obj.id
Copy link
Member

Choose a reason for hiding this comment

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

here too

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