• CatQuest joined the channel
      • agatzk joined the channel
      • elgranRoble has quit
      • monkey
        Shubh: Hi! I continued testing the series section, generally it's working better, but here's some more quirks i found, by order of importance:
      • • After checking "add works to series” and creating new series inline, works don’t appear below it and are not added to the series
      • • Apparently in some cases the relationship is only added to the series entity and not the works. I can’t reproduce the problem, but for example look at: https://test.bookbrainz.org/work/3ba0d95c-e9b3-... does not have a relationship "is part of” the series https://test.bookbrainz.org/series/78af1da7-caa... . The revision is https://test.bookbrainz.org/revision/30412
      • • If a user creates a new series inline then unchecks "add works to series” the new series does not appear in the submit recap, even if you re-chech and select the new series
      • • When selecting a series with existing items, in some cases the relationship order is not the same between existing series items and new items added: ("X contains Y" vs. “Y is part of X”). To reproduce, select existing series, uncheck and re-check "add works”, then select the series again. https://usercontent.irccloud-cdn.com/file/Lzf37...
      • • Not the most pressing: after unchecking and re-checking ‘add works to series', new Works is series section are "(unnamed)” and series is "New Entity” instead of their names https://usercontent.irccloud-cdn.com/file/ILB7b...
      • Shubh
        monkey: Hi, thanks for testing inline-series, really needed that
      • for the first point, latest commit should resolve that behavior.
      • about relationship on work, i think it should be automatically be added (reverse-relationship) when we add series-work rels, right?
      • monkey
        Not necessarily; the series and the work have different relationshipSets, which means you could add the relationship to one set and not the other
      • I thought I had updated test.bb to the latest commit, let me check that
      • monkey redeploys
      • Shubh
        Hmm so if we add rel on one, it doesn't effect the other entity (target /source)?
      • monkey
        Not automatically. We have some code that handles that, let me look for it
      • Right, it's in the ORM repo, there is a method called `updateRelationshipSets` that will compare relationship sets and created new ones for each concerned entity if needed: https://github.com/metabrainz/bookbrainz-data-j...
      • What I don't currently understand is that it seems to be working most of the time (relationship will be added to the series and to the work). This needs more testing to figure out in what circumstances the relationship doesn't get added.
      • Probably some more complicated interaction
      • Shubh
        test.bb seems to updated with latest commits, and i can see some issues with series section.
      • monkey
        I'm redeploying now just to be sure
      • I had a doubt, but it looks like it was a separate issue with my deployment setup.
      • Shubh
        monkey: same feeling, most of the time it automatically modify rel set on both entities. and this is i believe how normal entity editor works that is submitting single rel only on series.
      • monkey
        Yep. Somehow I did something that must have meant the Work wasn't part of the entities being processed.
      • Given an infinity of monkeys you can break any code ;p
      • Oh, here's another thing I just saw: The entity cards should probably have a max-width css style applied: https://usercontent.irccloud-cdn.com/file/bwTqD...
      • Shubh
        Ah still it shouldn't be breaking :<
      • monkey
        You know, you can't forget that at this point we're solving bugs for isolated parts of the system, which means the rest is working well :)
      • Shubh
        but still do you think it is fine if PR don't get merge before final code submission?, i think we should test this thoroughly before merging but not sure if it is possible before deadline.
      • monkey
        So since we are working off of a feature branch, we can keep working on the branch until we're happy with the result. The GSoC project doesn't require specifically that PRs be merged, just that the work was done
      • So don't worry too much about the deadline, you've been doing good work and we are not going to rush to the finish line :)
      • Of course I would immensely appreciate your continued help with polishing the project once we are past the deadline
      • Shubh
        Yep, definitely
      • also for the editor icon, do you know why there's no sparkles around magic wand on test.bb? or have any particular icon in mind to replace that?
      • monkey
        Not sure about the missing sparkles. My guess is that we used the wrong accentuation when we performed the magic ritual, but ¯\_(ツ)_/¯
      • I think your first choice (the book icon) was the way to go
      • Shubh
        open book icon looks kinda dull, though sparkled wand look good but it isn't working smh :< https://usercontent.irccloud-cdn.com/file/363fK...
      • monkey
        This would be less boring… :p ¯\_(ツ)_/¯
      • Looks like there are multiple wand icons; perhaps we're using the one without sparkles? https://usercontent.irccloud-cdn.com/file/ToNex...
      • Shubh
        It did show sparkled one on my local machine though but not on test.bb
      • monkey
        Maybe your local setup had a different package version installed?
      • Yes I think that's it: in FA 5.x the icon `faMagic` has sparkles: https://usercontent.irccloud-cdn.com/file/hUW40...
      • In version 6.x (which we updated to recently-ish) the icons are as above, with the icon being named differently too
      • Shubh
        Ahh nice catch monkey!
      • monkey
        There must be a fallback mechanism though so that importing faMagic still worked
      • Ah no, no fallback mechanism needed
      • Shubh
        Yep, now its fixed. thanks!
      • monkey
      • Shubh: I think I have an idea of what happened with the missing work-series relationship. I believe it is a timing-related conflict between the revisions.
      • For the work in question (3ba0d95c-e9b3-4ecd-b121-40acf830c361), have a look at the revisions, and then for each revision look at the relationship set id. The latest revision has a relSetId lower than the revision that came before, which seems odd.
      • Looking at the relationship sets, the relationship #38951 is the Work-Series relationship, so it was added, but then straight after that a new relationship set was created that overwrote it (without containing the just-added relationship) https://usercontent.irccloud-cdn.com/file/RWsMK...
      • Not sure if I'm making sense; let me know and I can write it up a bit clearer with more details
      • Conclusion: at some point in the process we should probably refresh the entity data (or maybe only the sets) to fetch newly-added data before continuing with the next step.
      • As to finding where exactly that is happening…that's another story
      • Do we concurrently process entities, or are they processed one after the other?
      • Shubh
        Ahh it might be just that
      • we do use Promise.all which is known to cause issues, let me make it sequential
      • monkey
        I think that might just be trick ! Although since I can't seem to be able to replicate the issue in any reliable manner, we'll probably have to wait and see.
      • But at least we're now off the trail of "it didn't create the relationship for the other entity" which was just a red herring from my part
      • Shubh
        monkey: any idea why error like `ErrorCtor [CustomError]: EmptyResponse` from bookshelf would occur, i've seen it several times and it seems very ambiguous since it doesn't cause any issue most of the time but sometime it do, not sure why.
      • Sequential calling seems to fixed the issue, also other issues related to works not added also fixed.
      • one thing left is that currently we just merge the old state with new state on server for old entities (https://github.com/tr1ten/bookbrainz-site/blob/...), this causes some issues for attributes like rels which couldn't directly can be merge. so do you think overriding rels (passing old rels along new ones) would be suitable here?
      • yvanzo_ joined the channel
      • Freso__ joined the channel
      • oj joined the channel
      • yvanzo has quit
      • Freso has quit
      • oj_ has quit
      • mayhem has quit
      • Freso__ is now known as Freso
      • yvanzo_ is now known as yvanzo
      • elgranRoble joined the channel