Skip to content
This repository has been archived by the owner on Sep 20, 2024. It is now read-only.

General: Fix links query on hero version #3900

Merged
merged 5 commits into from
Oct 10, 2022

Conversation

iLLiCiTiT
Copy link
Member

@iLLiCiTiT iLLiCiTiT commented Sep 23, 2022

Brief description

Fixed function to find links which crashed on hero versions.

Description

Don't lookup for hero versions in get_linked_representation_id because they don't have links. Made safer access to "data" key on found items to avoid possible crashes on items without links.

EDITED: Added query of hero version to use standard version to which is pointing.

Testing notes:

  1. Run Add to Site action on hero version
  2. It should not crash on finding linked representations

@iLLiCiTiT iLLiCiTiT self-assigned this Sep 23, 2022
@iLLiCiTiT iLLiCiTiT added the type: bug Something isn't working label Sep 23, 2022
@kalisp
Copy link
Member

kalisp commented Sep 23, 2022

Right mouse click on representation of Hero version in bottom right in Loader throws this:
Screenshot 2022-09-23 205406

Copy link
Member

@kalisp kalisp left a comment

Choose a reason for hiding this comment

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

See last comment.

Comment on lines +142 to +144
# Links are not stored to hero versions at this moment so filter
# is limited to just versions
"type": "version"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually - are we sure? The input link collection I believe is retrieved from containers in the scene - those I suspect would have the representation id of the hero version. As such by doing this "fix" we'd never be able to list those inputs even though they are stored?

Copy link
Member Author

@iLLiCiTiT iLLiCiTiT Sep 23, 2022

Choose a reason for hiding this comment

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

Links are added only to versioned version documents and hero version document don't have "data" at all.

But because hero versions are based on versioned documents we can reuse links from there.

Copy link
Collaborator

@BigRoy BigRoy Sep 23, 2022

Choose a reason for hiding this comment

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

Yes. Getting inputs FROM the hero version is then fixed. But any hero version IN the inputs is not? That hero version's inputs are not collected? (But might not be relevant for the UI we have now since it only shows direct inputs instead of a further depth.)

That does raise the question too that when we collect the inputs on publish would we want to resolve the hero version to the exact version it actually was at the time or do we want to just keep the link to the hero version? If we'd 'bake' it down to the exact version on collecting inputs the issue I mentioned would be gone plus you get the benefit of actually being able to see what version was used at the time in input link. Maybe we could tag it extra as "hero" so that the does know it originally was a hero link but at the time of publishing that was referring to v038.

@mkolar thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

Should we spin this up as a new issue?

@Tilix4
Copy link
Collaborator

Tilix4 commented Oct 5, 2022

Hi, is there anything to do for helping this fix to land?

@iLLiCiTiT iLLiCiTiT requested a review from kalisp October 5, 2022 11:09
@iLLiCiTiT iLLiCiTiT merged commit 1421b78 into develop Oct 10, 2022
@iLLiCiTiT iLLiCiTiT deleted the bugfix/fix_links_query_on_hero_version branch October 10, 2022 09:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants