search

UCL / frrant

2 stars 0 forks source link

Feature/323 add bibliography items directly #368

Closed rogerstafforducl closed 1 year ago

rogerstafforducl commented 1 year ago

Could I have a code review please? Probably not ready to pull yet, but: This is pretty much working now, but there are a couple of tests yet to write. Bibliography item is still listed on Antiquarian page (this is fine). Bib item can be created and edited from bibliography list, including associated antiquarians. There was a comment in the ticket that citing authors as well as antiquarians should be able to associate to bib items. I haven't done this, but it would be much easier than the antiquarian->bibliography link - to discuss, should this be done now or on another ticket? Many thanks, R

acholyn commented 1 year ago

I think associating the citing authors in another ticket is fine but considering there's no rush on closing this as Tom is on leave, it's also fine to include it in this ticket.

rogerstafforducl commented 1 year ago

Just noticed that I broke def antiquarians_and_authors_in_object_list(self, object_list): in views/search.py. Problem is that bibliography is no longer child of Antiquarian, but an entity in its own right and can be associated with citing authors as well. One solution would be to handle it in this function in the same way as Antiquarians and Authors (generate and display sorted list of bibliography items), but then the function name would be a bit of a misnomer - perhaps that wouldn't matter too much, or perhaps it would be ok to rename the function? @acholyn I think you're familiar with this file, could you advise please?

acholyn commented 1 year ago

Should be fine to change the function and its name so it's more appropriate, I only see it being called one other time in the same file so not too much updating on that front

rogerstafforducl commented 1 year ago

When searching for plain text in the example here a search succeeds, when searching in italicised text nothing is returned - I can't see why. E.g. (from below) "The early historians" will work, as will "London", but "Latin Historians" returns nothing. I don't think this has anything to do with changes relating to the Bib model in search, so perhaps this should be another issue? image

rogerstafforducl commented 1 year ago

Unable to loaddata because of migrations. After reversing the migrations, still unable to loaddata. bit stuck :( . Antiquarian model seems to be using a PG intermediate table research_citingauthor_bibliography_items - seems to have got tangled wrt the migrations 61,62,63. Need to fix it.