#bookbrainz

monkey: I print out the seriesDiffs but i can't see the diff for attributeSet in this array https://usercontent.irccloud-cdn.com/file/xcyCW...

Right, that's issue #1 with the diffs, then.

To be honest I don't see why it's not working

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-j...

Bit of ashot in the dark, but…

And you could also have a look at the value of the jsonified baseData and otherData here: https://github.com/bookbrainz/bookbrainz-data-j...

okay let me look.

> you could see what the printout looks like if you make this method simply `return true`

it outputs the same result as before.

This is how the jsonified output of otherdata and basedata looks like https://usercontent.irccloud-cdn.com/file/ks7Rh...

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?

maybe i can test it again by adding this line to workRevision diff `relationshipSet.relationships.attributeSet.relationshipAttributes.value`

Ah, if it's not there that would 100% explain it

I thought you were using a series revision :p

So, yeah, that's definitely what it is

Otherwise these related models are not loaded

It ends up in `withRelated: includes` in the diffRevisions method

`const baseDataPromise = base.related('data').fetch({require: false, withRelated: includes});`

So this line `relationshipSet.relationships.attributeSet.relationshipAttributes.value` should probably be added to each entity revision type

> I thought you were using a series revision :p

I am using series revision🤔

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-...

Unless I'm missing something

A Pail of Air is a relationship within that series entity.

Right, but the info I see is that of a workRevision, not a seriesRevision

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

Although now i haved added this code `relationshipSet.relationships.attributeSet.relationshipAttributes.value` to each models

but it still yields the same result

no attributeSet in diff.

I'll try locally

okay :)

So I finally got to it, and I do see the attributes changes in the diff. Let me take a screenshot

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.

You'll see that attributeSetId has changed, along with the relationshipAttributes and their value

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

Here's what I added to bookbrainz-site's src/server/routes/revison.js file:

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.

So I think maybe you were only looking at the workRevision diffs and not the seriesRevision diffs

oooooh yess 🤦

It is indeed showing the diffs for attributes. My bad i never tested it by editing the relationship.

I was only checking for newly created entity. https://usercontent.irccloud-cdn.com/file/3DHoD...

Thank you so much 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

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

> if values on both sides are null (for example relationships with no attributes = null attributeSet) it won't show anything

I guess even we add a new relationship( which have attributes value )[Kind ="n")] the diff doesn't include the path attributeSet.

Exactly

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

And same if you then change the attribute value: new attributeSetID and attribute value will show in the diff

But for newly added relationship(with attributes)[Kind="n"] will also have attributeSetId isn't it ?

I'm not sure currently the attributeSetId will show up in the diffs

Here's the diffs I see for a revision where I added a new relationship to a series: https://usercontent.irccloud-cdn.com/file/ilbnC...

column 3 right ?

Column 3?

Not sure I understand

sorry i mean, index 3 of second table

it shows new relationship is added

Yes

And so the relationshipSet will have a relationship which will have an attributeSetId

Here's what the relationships array looks like: https://usercontent.irccloud-cdn.com/file/AbIdx...

YEss

Note the attributeSetId, confirming this relationship does have attributes

(relationshipAttributes is collapsed but it should have the attributes and their values)

So there will be some modifications needed in how we display new relationships too, in order to also show the attributes

Yeah Got it.

That would be in the `formatRelationshipAdd` method of the diffFormatters I'm guessing

Or `formatAddOrDeleteRelationshipSet`

Or both. I'm not so sure at this point :p

Okay thanks. I will look into it. Will ping you if i have any doubts :p

👍