#bookbrainz

/

      • akashgp09 joined the channel
      • spider joined the channel
      • yvanzo has quit
      • yvanzo joined the channel
      • spider has quit
      • matterharz joined the channel
      • matterharz has quit
      • monkey has quit
      • monkey joined the channel
      • aphelion joined the channel
      • monkey
        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
      • BrainzBot
        You're doing good work, BenOckmore!
      • 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
      • monkey
        I think so too. "Focusing on Facebook developers' needs" does not sound like a perennial way forward
      • akashgp09
        monkey: how can i test this model `relationship_attribute_text_value` 🤔
      • monkey
        I need more context, or a more detailed question :)
      • akashgp09
        How can i insert into values to `relationship_attribute_text_value` when creating a relationshipAttributeSet.
      • monkey
        Sorry, I'll be back very shortly, got someone ringing the door
      • akashgp09
        yep np : )
      • yvanzo has quit
      • 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
      • monkey
        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?
      • BrainzGit joined the channel
      • akashgp09
        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`
      • monkey
        Something like `new RelationshipAttributeTextValue({id: myRelAttributeId, value: "Hello!"}).save()` ?
      • `new RelationshipAttributeTextValue({id: myRelAttributeId, value: "Hello!"}).save(null, {method: 'insert'});` to be more exact
      • akashgp09
        ye, how can i insert multiple rows at once ?
      • monkey
        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)
      • akashgp09
        yeah thanks for this : )
      • monkey
        👍
      • akashgp09
        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 ?
      • monkey
        `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})`.
      • akashgp09
        > 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
      • monkey
        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
      • akashgp09
        await new RelationshipAttributeTextValue({id: row.id value: 'someValue' }).save(null, {method: 'insert'})) will this always be a insert method ?
      • monkey
        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
      • akashgp09
        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 ?
      • monkey
        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
      • akashgp09
        but if text_value isnt' in the relationship attribute set so how will it know if something has changed ?
      • monkey
        We'll need to load the text value accordingly before comparing the attributes
      • akashgp09
        ok understandable
      • monkey
        So somewhere we'll load the RelationshipAttribute `withRelated: ['value']` , something along those lines
      • akashgp09
        i was thinking that
      • monkey
        Or if it's loaded before that, the `withRelated: attributeSet.attribute.value` I mentionned earlier
      • Here we go, it's in `getMasterRelationshipSetForEntity``
      • akashgp09
        we can keep date attribute inside relatinship table just like MB and avoid this complicacy ?
      • monkey
        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
      • akashgp09
        Cool Thanks : )
      • monkey
        Does it all make a bit more sense now?
      • It's all quite convoluted
      • akashgp09
        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
      • monkey
        Can't blame you! I had a hard time putting it all together myself
      • akashgp09
        :P 😅
      • so i will now write the test for RelationshipAttributeTextValue then i guess we can have a code review 😁
      • yvanzo joined the channel
      • shivam-kapila has quit
      • shivam-kapila joined the channel
      • shivam-kapila has quit
      • shivam-kapila joined the channel
      • akashgp09 has quit