CatQuest: The wrong identifier issue preventing editing/merging is a problem I've started to solve. Basically, the website won't let you submit wrong information, and that includes previously-entered wrong information. I've been working on fixing the wrong identifiers across the database, it's just taking some time :)
2020-05-28 14902, 2020
Mr_Monkey
In the baby shakespeare case, an OL book ID = BB Edition. OL Work ID = BB Work. I'm changing this one accordingly
2020-05-28 14917, 2020
Chinmay3199 has quit
2020-05-28 14939, 2020
Mr_Monkey
Done.
2020-05-28 14917, 2020
Mr_Monkey
Regarding the merge target selection, I think it makes sense to have it in only one place, and the merge queue seems appropriate. Now that you know it's there, does it still bother you?
2020-05-28 14917, 2020
Mr_Monkey
What if there was a help text in the merge page header (under the names of the entities) like "To change which entity to merge into, click on its ^^ icon in the merge queue, at the bottom of the page" ?
2020-05-28 14923, 2020
CatQuest
I've been working on fixing the wrong identifiers across the database, it's just taking some time :)
2020-05-28 14923, 2020
CatQuest
oh a report of these would be helpful as I could help fix them
2020-05-28 14950, 2020
iliekcomputers
alastairp: while you're at it, can you take a look at my LB PR too :D
2020-05-28 14900, 2020
alastairp
iliekcomputers: it's the next open tab!
2020-05-28 14903, 2020
CatQuest
as for the merge thing. honestly this is going to be something leading to people merging to the "unintended" bbid ALL THE TIME
2020-05-28 14903, 2020
CatQuest
so yes a help text is absolute must
2020-05-28 14918, 2020
jmp_music has quit
2020-05-28 14925, 2020
iliekcomputers
Yay! Thanks!
2020-05-28 14943, 2020
CatQuest
but it's still vaugly unintuitive not to be doing it *on* the merging screen and *on* the merging screen it would make much more sense to have it up top :)
2020-05-28 14949, 2020
ishaanshah
Mr_Monkey: thanks :D
2020-05-28 14916, 2020
CatQuest
I aknowledge that I'm only one person. but my gut insticknt is that it's a bad placement for it (though I agree with having this ability only one place)
2020-05-28 14912, 2020
jmp_music joined the channel
2020-05-28 14923, 2020
Mr_Monkey
I don't necessarily disagree. it would make sense in the top of the merge screen. I just don't currently have a good idea for how to put that in a way that is both usable and looks good
2020-05-28 14935, 2020
CatQuest
right now usability > looks good
2020-05-28 14955, 2020
CatQuest
we can always get more design stuff in later
2020-05-28 14919, 2020
CatQuest
utility *always* trumps looks
2020-05-28 14925, 2020
CatQuest
(unless you by looks mean "organized" or "ubersichtclich")
I'm not sure what's so bad about the radio buttons we use in MB :p
2020-05-28 14910, 2020
Mr_Monkey
A big "meh" from me.
2020-05-28 14936, 2020
reosarevok
I mean, maybe, but I find it much more clear than that image, where I'm not sure what the arrow even means. Does it mean the second is to be merged into the first? And if so, how would it work if there's 3 or 4 works?
2020-05-28 14920, 2020
Mr_Monkey
I agree, it's not clear.
2020-05-28 14929, 2020
shivam-kapila
alastairp: Hi thanks for the review. I was already onto some optimisations and your review helped me see a clear view to it. I had a doubt. For the GET APIs we also intend to send the `total_count`. So if I paginate in the DB would I need to make a separate call for total count to the DB or there is some better workaround?
most of the time if I was to do something like this I would just make 2 queries
2020-05-28 14913, 2020
alastairp
this is one of the cases where I don't think 2 queries would be a problem. You're getting two different types of data, so that's OK
2020-05-28 14928, 2020
alastairp
in the case of the feedback/username you're getting _the same_ information twice
2020-05-28 14901, 2020
shivam-kapila
yeah I also felt that when writing tests. Thanks
2020-05-28 14903, 2020
alastairp
shivam-kapila: also, as something to keep in mind, you never want to send more data than necessary from the database to the server. in this case it might be OK, but imagine if you were doing the same thing for the listens that a person had, and you had to retrieve 200,000 rows, only to throw away 199,950 of them
2020-05-28 14903, 2020
shivam-kapila
I totally agree. That's unnecessary. I had been slow for 2-3 days because I wanted to optimise things but I had diverting thoughts in mind. e.g. the get_by_user_id and get_by_user_id_and_score can be easily combined
2020-05-28 14958, 2020
alastairp
đź‘Ť no problem. this is why we do the reviews
2020-05-28 14920, 2020
alastairp
keep in mind also that I know most of this because I've gotten it wrong at least once before
2020-05-28 14901, 2020
shivam-kapila
Yeah. Experience helps a lot. I would have discussed but I wasn't clear about some things. Thanks :)
2020-05-28 14951, 2020
shivam-kapila
1 thing more. we do `Feedback(**dict(row))` for rows fetched from DB and then convert it back to dicts. The Feedback step seems useful for valedictory purposes. Should I keep it or just pass back dicts directly?
2020-05-28 14925, 2020
bitmap
zas: should I stop search-server on yehudi? I don't see any connections to it in netstat
2020-05-28 14950, 2020
zas
I think you can, there's another instance at least
2020-05-28 14901, 2020
bitmap
cool
2020-05-28 14931, 2020
diru1100
yvanzo: Hey, I created a fake account to see what all a spam account can do.
2020-05-28 14924, 2020
alastairp
shivam-kapila: where do you do that? in the db module?
oh, and then you mean that you convert back to dicts in the API to send the response?
2020-05-28 14912, 2020
shivam-kapila
yes
2020-05-28 14941, 2020
alastairp
right - that's not strictly correct though. You _serialise it to JSON_ because that's the API format
2020-05-28 14947, 2020
alastairp
the difference is subtle, but important
2020-05-28 14958, 2020
diru1100
yvanzo: No, I created it in production server online. Will delete after I get enough details.
2020-05-28 14905, 2020
alastairp
the idea is that everywhere in the application we should use the models
2020-05-28 14919, 2020
diru1100
yvanzo: spam account should be verified to post any content/ rating IIRC. and do the spam accounts post useless content or just fill editor profile details with spam?
2020-05-28 14928, 2020
alastairp
conversion to/from the models happen at the "edge" of the application (where it connects to external services - a database, or a client)
[16:16] <Mr_Monkey> With the icon a bit more obviously clickable
2020-05-28 14926, 2020
CatQuest
[16:17] <reosarevok> I'm not sure what's so bad about the radio buttons we use in MB :p
2020-05-28 14926, 2020
CatQuest
well that's an improvement. but yes. radio buttins +1
2020-05-28 14929, 2020
CatQuest
button*
2020-05-28 14915, 2020
Mr_Monkey
I'm trying out having a dropdown to select the merge target
2020-05-28 14932, 2020
CatQuest
uhh..
2020-05-28 14935, 2020
Mr_Monkey
Radio buttons make sense if you have an action afterwards, which in the case of BB isn't the case
2020-05-28 14946, 2020
CatQuest
the ation here is "merge" monkey
2020-05-28 14916, 2020
Mr_Monkey
Well, it doesn't exactly work like MB though. if you change the merge target, you need to reload the merge page
2020-05-28 14920, 2020
CatQuest
I'd honestly prefer radio to dropdown
2020-05-28 14935, 2020
CatQuest
hm. yes. you need to figure out that
2020-05-28 14906, 2020
CatQuest
(it shouldn't reload the page imho)
2020-05-28 14934, 2020
Mr_Monkey
Can't get around that I'm afraid
2020-05-28 14904, 2020
CatQuest
.. well atleast then you'll need to display more informtion
2020-05-28 14929, 2020
CatQuest
no the line of things to merge , to be able to see wich one is the oen you want, at a glance
2020-05-28 14914, 2020
CatQuest
(basically to see "which one is the most/most empty/oldest revision id containing, etc)
2020-05-28 14907, 2020
CatQuest
but the image you showed is an improvment and i'd rather have that than a dropdown (dropdowns are annoying af, because yo umight accientally select the wrong one or accidentally scroll in it or whatnot)
2020-05-28 14931, 2020
Mr_Monkey
Re: showing more information, considering all the information from the various entities is collated and presented on the same page, is that necessary here?
2020-05-28 14957, 2020
Mr_Monkey
Again it totally makes sense in the MB way of merging, but maybe less so for BB?
2020-05-28 14911, 2020
yvanzo
diru1100: I encourage you to set your own server up and use it for test instead.
At least in cases where the name is the same but other things differ
2020-05-28 14948, 2020
reosarevok
Otherwise, how would I know what to pick? Or should we pick just on name, in which case why not just always pick the smaller ID and also allow picking the preferred name
Int's an interactive merge, the page presents multiple choices (dropdown) when data is different between entities
2020-05-28 14932, 2020
Mr_Monkey
As for how to know which entity to select, fair point. There are links to each in the header but bbid isn't shown.
2020-05-28 14951, 2020
yvanzo
diru1100: there is no general rule about spammers, they can do it all or just one specific type of spam.
2020-05-28 14953, 2020
Mr_Monkey
That being said, not sure how necessary it is.
2020-05-28 14911, 2020
reosarevok
Mr_Monkey: looking at that form, I'd just go with the smallest ID
2020-05-28 14928, 2020
Mr_Monkey
smallest… BBID?
2020-05-28 14937, 2020
reosarevok
DB ID (older entry)
2020-05-28 14950, 2020
reosarevok
Seems like you can pick everything while merging, so what is the use of having the user pick the destination?
2020-05-28 14928, 2020
Mr_Monkey
They don't have a DB id. They do have revision ids each, and i could perhaps try to select the one that has the lowest revision id.
2020-05-28 14940, 2020
Mr_Monkey
Ask CatQuest I guess :)
2020-05-28 14923, 2020
Mr_Monkey
There's one thing: the item you merge into has its info preselected in the dropdowns
2020-05-28 14906, 2020
Mr_Monkey
So if you know you trust the info in entity A it makes sense to have it as the target instead of having to change in each dropdown to select entity A's info…
2020-05-28 14933, 2020
diru1100
yvanzo: Roger that, will set up in mac ASAP. Ig the the dev version will do?
I could show the BBID in the dropdown, that would make it clearer, but you'd still have to know which BBID you want.
2020-05-28 14937, 2020
alastairp
iliekcomputers: done!
2020-05-28 14950, 2020
diru1100
Thanks :)
2020-05-28 14915, 2020
iliekcomputers
Thanks alastairp!
2020-05-28 14912, 2020
ishaanshah
iliekcomputers: Hi!
2020-05-28 14934, 2020
iliekcomputers
Hey!
2020-05-28 14902, 2020
alastairp
many many years ago I did a lot of work on data modeling, and how to name things. I've forgotten a lot of that, might be something interesting to go back and address
2020-05-28 14904, 2020
iliekcomputers
I skimmed the artist frontend PR. It seemed reasonable to me. I'll take a look again today
