#bookbrainz

Looks like moving from Flow to Typescript came at just about the right time: https://medium.com/flow-type/clarity-on-flows-d...

!m BenOckmore

You're doing good work, BenOckmore!

Hmm although that post just makes it sound like they're making Flow more like TypeScript, while being less focused on the open source community

Does make me feel that it was the right choice switching though

I think so too. "Focusing on Facebook developers' needs" does not sound like a perennial way forward

monkey: how can i test this model `relationship_attribute_text_value` 🤔

I need more context, or a more detailed question :)

How can i insert into values to `relationship_attribute_text_value` when creating a relationshipAttributeSet.

Sorry, I'll be back very shortly, got someone ringing the door

yep np : )

Check line 46 of this file https://github.com/bookbrainz/bookbrainz-data-j...

Suppose i have inserted two rows ( id: 1, attribute_id: 1 ) , ( id: 2, attribute_id: 2) in the `relationship_attribute` table

next i want to insert the attribute values associated with these two id's : 1 and 2 in the table `relationship_attribute_text_value table`

> ( id: 1, attribute_id: 1 ) , ( id: 2, attribute_id: 2) sorry i meant attribute_type here not attribute_id

akashgp09: We'll need some utility functions to update the relationship_attribute_set elements, like we do for other sets. Have a look for example at https://github.com/bookbrainz/bookbrainz-data-j... , as well as this file with some set-realted utility functions:

Ah, never mind, I see you alrady have those defined here https://github.com/bookbrainz/bookbrainz-data-j...

So I'm not sure I understand specifically what the question is. Where are you blocked?

after inserting some rows in the `relationship_attribute` table how will i insert the attribute values associated with this rows (id) in the table `relationship_attribute_text_value`

Something like `new RelationshipAttributeTextValue({id: myRelAttributeId, value: "Hello!"}).save()` ?

`new RelationshipAttributeTextValue({id: myRelAttributeId, value: "Hello!"}).save(null, {method: 'insert'});` to be more exact

ye, how can i insert multiple rows at once ?

I guess I would call the method twice. Not sure if there's an option for saving multiple new rows

Let me look

No great options it seems. I would recommend calling `await new RelationshipAttributeTextValue(…).save(…)` twice, or use the underlying knex library if for some reason that doesn't work (see https://github.com/bookshelf/bookshelf/issues/445)

yeah thanks for this : )

👍

actually the problem is something bigger. give me some time I will write this up and explain the issue properly . Right now what i can think of is that it's not appropriate to add foreign on `relationship_atrribute` table id column

instead we should have the text_value column in the same table `relationship_atrribute` table

also no other table `relationship` , `aliases' , `release_event`, `identifiers` have foreign key on their `id` column

monkey: give this a read what do you think ?

`attribute.map(row => await new RelationshipAttributeTextValue({id: row.id value: 'someValue' }).save(null, {method: 'insert'}));` looks pretty good to me at first glance.

The textvalue id ends up equal to the attribute id, so we should be able to do an SQL JOIN on that id.

When the value changes, we also create a new attribute row and a new attributeSet row.

The issue with putting the text value in the relationship_attribute table is that an attribute's value will not always be text, for example for dates.

Now we do need to make sure that we can retrieve the value easily, say with `Relationship.forge(…).fetch({withRelated: attributeSet.attribute.value})`.

> The textvalue id ends up equal to the attribute id, so we should be able to do an SQL JOIN on that id.

the joining part is OK

but i am conern with how will we know which attribute id will have which textValue

one more thing

Ah, I see maybe where the misunderstanding is: the relationship_attribute_text_value has no column `id` but instead the column is `attribute_id` with a foreign key pointing to an attribute

await new RelationshipAttributeTextValue({id: row.id value: 'someValue' }).save(null, {method: 'insert'})) will this always be a insert method ?

So when we join, we join on the attribute_id

I think we'll only ever insert new items (if the value is new or if the value changes)

That's how we keep the history of modifications

Whenever an attribute value changes, we create a new attribute (we keep the attributes that haven't changed as-is ), put them all in a new attributeSet and create a new relationship with that new attributeSet, and finally a new relationshipSet with the new relationship (and any previous rels that have not changed)

It's a bit involved but I don't see another solution if we want to keep revision history like we do with the rest of the sets

For identifier https://github.com/bookbrainz/bookbrainz-data-j... this function is responsible for validating if some has changed in the set or not right ?

Similary in our case updateRelationshipAttributeSet ?

Yes, that's it !

There is some functionality that is common for all sets, which is why this `comparisonFunc` is created that is specific for each type of set items

See https://github.com/bookbrainz/bookbrainz-data-j...

but if text_value isnt' in the relationship attribute set so how will it know if something has changed ?

We'll need to load the text value accordingly before comparing the attributes

ok understandable

So somewhere we'll load the RelationshipAttribute `withRelated: ['value']` , something along those lines

i was thinking that

Or if it's loaded before that, the `withRelated: attributeSet.attribute.value` I mentionned earlier

Here we go, it's in `getMasterRelationshipSetForEntity``

we can keep date attribute inside relatinship table just like MB and avoid this complicacy ?

Something like `withRelated: ['relationships.attributeSet.attributes.value']`

For the date, that's a possibility. I have to think about it a bit more, but in any case you should not preocupy yourself with the dates at this time

(Or maybe at all :) )

We can implement relationship dates when we come to it, and by that time I'll have a better idea about how to do it

Cool Thanks : )

Does it all make a bit more sense now?

It's all quite convoluted

I have understand this well. just had a bit confusion with insertion part in table relationship_attributes_text_value

but i guess we will go with looping then

Can't blame you! I had a hard time putting it all together myself

:P 😅

so i will now write the test for RelationshipAttributeTextValue then i guess we can have a code review 😁