#bookbrainz

/

      • ZaphodBeeblebrox joined the channel
      • 2021-07-01 18212, 2021

      • ZaphodBeeblebrox has quit
      • 2021-07-01 18212, 2021

      • ZaphodBeeblebrox joined the channel
      • 2021-07-01 18246, 2021

      • akashgp09 joined the channel
      • 2021-07-01 18219, 2021

      • CatQuest has quit
      • 2021-07-01 18218, 2021

      • akashgp09
        monkey: I print out the seriesDiffs but i can't see the diff for attributeSet in this array https://usercontent.irccloud-cdn.com/file/xcyCWMn…
      • 2021-07-01 18204, 2021

      • monkey
        Right, that's issue #1 with the diffs, then.
      • 2021-07-01 18234, 2021

      • monkey
        To be honest I don't see why it's not working
      • 2021-07-01 18220, 2021

      • monkey
        Out of curiosity, you could see what the printout looks like if you make this method simply `return true`, that could possibly give us a clue https://github.com/bookbrainz/bookbrainz-data-js/…
      • 2021-07-01 18226, 2021

      • monkey
        Bit of ashot in the dark, but…
      • 2021-07-01 18244, 2021

      • monkey
        And you could also have a look at the value of the jsonified baseData and otherData here: https://github.com/bookbrainz/bookbrainz-data-js/…
      • 2021-07-01 18258, 2021

      • akashgp09
        okay let me look.
      • 2021-07-01 18202, 2021

      • akashgp09
        > you could see what the printout looks like if you make this method simply `return true`
      • 2021-07-01 18221, 2021

      • akashgp09
        it outputs the same result as before.
      • 2021-07-01 18231, 2021

      • akashgp09
        This is how the jsonified output of otherdata and basedata looks like https://usercontent.irccloud-cdn.com/file/ks7RhwA…
      • 2021-07-01 18202, 2021

      • monkey
        I'm just seeing now that this is a Work revision. Is it possible that there is currently no way to add rel attributes for these relationships, and so the attributesSet value woudl be null, and not shown in the diff?
      • 2021-07-01 18231, 2021

      • akashgp09
        maybe i can test it again by adding this line to workRevision diff `relationshipSet.relationships.attributeSet.relationshipAttributes.value`
      • 2021-07-01 18259, 2021

      • monkey
        Ah, if it's not there that would 100% explain it
      • 2021-07-01 18207, 2021

      • monkey
        I thought you were using a series revision :p
      • 2021-07-01 18222, 2021

      • monkey
        So, yeah, that's definitely what it is
      • 2021-07-01 18248, 2021

      • monkey
        Otherwise these related models are not loaded
      • 2021-07-01 18251, 2021

      • monkey
        It ends up in `withRelated: includes` in the diffRevisions method
      • 2021-07-01 18206, 2021

      • monkey
        `const baseDataPromise = base.related('data').fetch({require: false, withRelated: includes});`
      • 2021-07-01 18239, 2021

      • monkey
        So this line `relationshipSet.relationships.attributeSet.relationshipAttributes.value` should probably be added to each entity revision type
      • 2021-07-01 18256, 2021

      • akashgp09
        > I thought you were using a series revision :p
      • 2021-07-01 18212, 2021

      • akashgp09
        I am using series revision🤔
      • 2021-07-01 18249, 2021

      • monkey
        Looking at the jsonified data you posted, it looks like the diff for a revision of this entity: https://bookbrainz.org/work/08bb8a6b-202b-4a9c-b4…
      • 2021-07-01 18257, 2021

      • monkey
        Unless I'm missing something
      • 2021-07-01 18247, 2021

      • akashgp09
        A Pail of Air is a relationship within that series entity.
      • 2021-07-01 18230, 2021

      • monkey
        Right, but the info I see is that of a workRevision, not a seriesRevision
      • 2021-07-01 18212, 2021

      • monkey
        To be clear, the revision in question will affect both the series and work entities, so for the same rev number you'll have a workRevision and a seriesRevision
      • 2021-07-01 18218, 2021

      • akashgp09
      • 2021-07-01 18243, 2021

      • akashgp09
        Although now i haved added this code `relationshipSet.relationships.attributeSet.relationshipAttributes.value` to each models
      • 2021-07-01 18206, 2021

      • akashgp09
        but it still yields the same result
      • 2021-07-01 18213, 2021

      • akashgp09
        no attributeSet in diff.
      • 2021-07-01 18240, 2021

      • monkey
        I'll try locally
      • 2021-07-01 18248, 2021

      • akashgp09
        okay :)
      • 2021-07-01 18232, 2021

      • monkey
        So I finally got to it, and I do see the attributes changes in the diff. Let me take a screenshot
      • 2021-07-01 18259, 2021

      • monkey
      • 2021-07-01 18236, 2021

      • monkey
        So the top table is just the diff change paths, equivalent to the middle column of the second table (path) since there wasn't enough space to display it.
      • 2021-07-01 18219, 2021

      • monkey
        You'll see that attributeSetId has changed, along with the relationshipAttributes and their value
      • 2021-07-01 18207, 2021

      • monkey
        In the second table, the last two columns (lhs and rhs / left-hand side and right-hand side) show the value before and after the revision
      • 2021-07-01 18216, 2021

      • monkey
        Here's what I added to bookbrainz-site's src/server/routes/revison.js file:
      • 2021-07-01 18231, 2021

      • monkey
      • 2021-07-01 18236, 2021

      • monkey
        So everything seems to be working okay as far as I can tell. The changes do need to be formatted and all that, but at least the diff has the appropriate changes I'm expecting to see.
      • 2021-07-01 18213, 2021

      • monkey
        So I think maybe you were only looking at the workRevision diffs and not the seriesRevision diffs
      • 2021-07-01 18215, 2021

      • akashgp09
        oooooh yess 🤦
      • 2021-07-01 18237, 2021

      • akashgp09
        It is indeed showing the diffs for attributes. My bad i never tested it by editing the relationship.
      • 2021-07-01 18252, 2021

      • akashgp09
        I was only checking for newly created entity. https://usercontent.irccloud-cdn.com/file/3DHoD9u…
      • 2021-07-01 18240, 2021

      • akashgp09
        Thank you so much monkey.
      • 2021-07-01 18253, 2021

      • monkey
        Right. the diffs only show changes, and if values on both sides are null (for example relationships with no attributes = null attributeSet) it won't show anything
      • 2021-07-01 18233, 2021

      • monkey
        No worries, at least we know this part of the diff works as expected, and while in there we've discovered that the entityRevisions all need to have that `relationshipSet.relationships.attributeSet.relationshipAttributes.value`, that's good progress
      • 2021-07-01 18214, 2021

      • akashgp09
        > if values on both sides are null (for example relationships with no attributes = null attributeSet) it won't show anything
      • 2021-07-01 18222, 2021

      • akashgp09
        I guess even we add a new relationship( which have attributes value )[Kind ="n")] the diff doesn't include the path attributeSet.
      • 2021-07-01 18230, 2021

      • monkey
        Exactly
      • 2021-07-01 18203, 2021

      • monkey
        If you then add an attribute, there will be an attributeSet with an id , that id will be ≠ null and will show in the diffs
      • 2021-07-01 18245, 2021

      • monkey
        And same if you then change the attribute value: new attributeSetID and attribute value will show in the diff
      • 2021-07-01 18252, 2021

      • akashgp09
        But for newly added relationship(with attributes)[Kind="n"] will also have attributeSetId isn't it ?
      • 2021-07-01 18245, 2021

      • monkey
        I'm not sure currently the attributeSetId will show up in the diffs
      • 2021-07-01 18246, 2021

      • monkey
        Here's the diffs I see for a revision where I added a new relationship to a series: https://usercontent.irccloud-cdn.com/file/ilbnCEr…
      • 2021-07-01 18222, 2021

      • akashgp09
        column 3 right ?
      • 2021-07-01 18202, 2021

      • monkey
        Column 3?
      • 2021-07-01 18208, 2021

      • monkey
        Not sure I understand
      • 2021-07-01 18233, 2021

      • akashgp09
        sorry i mean, index 3 of second table
      • 2021-07-01 18217, 2021

      • akashgp09
        it shows new relationship is added
      • 2021-07-01 18221, 2021

      • monkey
        Yes
      • 2021-07-01 18242, 2021

      • monkey
        And so the relationshipSet will have a relationship which will have an attributeSetId
      • 2021-07-01 18201, 2021

      • monkey
        Here's what the relationships array looks like: https://usercontent.irccloud-cdn.com/file/AbIdxhQ…
      • 2021-07-01 18203, 2021

      • akashgp09
        YEss
      • 2021-07-01 18237, 2021

      • monkey
        Note the attributeSetId, confirming this relationship does have attributes
      • 2021-07-01 18203, 2021

      • monkey
        (relationshipAttributes is collapsed but it should have the attributes and their values)
      • 2021-07-01 18245, 2021

      • monkey
        So there will be some modifications needed in how we display new relationships too, in order to also show the attributes
      • 2021-07-01 18252, 2021

      • akashgp09
        Yeah Got it.
      • 2021-07-01 18259, 2021

      • monkey
        That would be in the `formatRelationshipAdd` method of the diffFormatters I'm guessing
      • 2021-07-01 18238, 2021

      • monkey
        Or `formatAddOrDeleteRelationshipSet`
      • 2021-07-01 18246, 2021

      • monkey
        Or both. I'm not so sure at this point :p
      • 2021-07-01 18206, 2021

      • akashgp09
        Okay thanks. I will look into it. Will ping you if i have any doubts :p
      • 2021-07-01 18219, 2021

      • monkey
        👍
      • 2021-07-01 18233, 2021

      • akashgp09 has quit