Fix : BB #348/340

[prabalsingh24]

Problem

Revision page was showing duplicate entities.
https://tickets.metabrainz.org/browse/BB-340

Solution

I added merge:false, remove:false in model.fetchAll()..
Also changed bookbrainz-data-js's version in package.json from 2.3 to 2.5. 2.3's package.json had bookshelf@0.13 as it's required dependency and remove:false;merge:false fix came in 0.14 version.
Also changed package-json.lock accordingly. It was updated automatically :P

[coveralls]

Coverage remained the same at 42.906% when pulling d5e4a14 on prabalsingh24:fix-BB-348/340 into 9f92b62 on bookbrainz:master.

[MonkeyDo]

Hmm, I think we're in an undesirable situation where the update to the ORM (-data-js) requires an update of various parts of the web server.
The reason is that an default option changed, and now if you don't pass require: false to a .fetch or .fetchAll call, you'll get an error thrown if there are no results.
The errors in questions are fairly easy to manage, but the codebase needs a lot of small changes.

A similar situation will prevent me from merging #339 straight away: the fetchPage-related change on the ORM will not be necessary once we use the newer version of the ORM (it comes included by default)

The situation that is problematic is that I made all the necessary modifications as part of developing the merge tool, and all that work is in PR #333

Our options are:

1. wait until I merge 333, which will bring the same fix you did on this PR 345
2. I try to pry apart the ORM update related commits out of 333 and make a new PR out of that

I was hoping I'd have the time to get 333 finished and it wouldn't be a problem, but I was wrong. I'll look at implementing option 2 to allow us to move forward

[prabalsingh24]

Sorry I thought I was closing the comment, accidentally closed the PR. There is another solution, instead of using bookshelf I can write raw-sql query, that will work

[prabalsingh24]

What do you think? @MonkeyDo

[MonkeyDo]

instead of using bookshelf I can write raw-sql query, that will work. What do you think?

I think that would work, but there's duplication of efforts there that I'm not fond of.
After a quick test, I do believe using PR 333 the problems of tickets 348/340 are solved.

Sorry, I should have realized earlier that it would solve these issues. In any case, let's leave this PR open until it is all figured out.

[MonkeyDo]

OK, I wasn't as stupid as I thought, and I did almost all of the ORM update work on a separate branch, so I'm doing a little bit of cherry picking, some cleaning and some testing, and I'll open a new PR (currently draft #346 ).
Once that is merged we'll be able to come back to this PR

[prabalsingh24]

Haha ofcourse :) Yeah alright. Let's look at this once that is merged :D

[MonkeyDo]

I merged the master branch and resolved the small conflicts that popped up, and tested again, that's working great !

[endurance21]

@MonkeyDo
@prabalsingh24
Is there something left to work upon here??

[prabalsingh24]

@MonkeyDo
@prabalsingh24
Is there something left to work upon here??

No it's been merged.