Pkp-lib: [OMP] Only one featured monograph is displayed

[alexdryden]

Version: OMP3.2.0-3

Issue: Featured monographs share the same order index (1) by default unless they are explicitly reordered by the user. As a result only one featured monograph is displayed.

Steps to reproduce:

1. Start with a few books and make none of them featured.
2. Select checkboxes to make them all featured.
3. Select "View Site" and only one title is listed as featured

Here is a diff result for the dbdump before and after explicitly ordering. First one (<) is before, second one (>) is after. The last value for each entry is the the order index, seq.

< INSERT INTO `features` VALUES (3,512,2,1),(4,512,2,1),(5,512,2,1);
---
> INSERT INTO `features` VALUES (3,512,2,2),(4,512,2,1),(5,512,2,0);

[NateWr]

Thanks @alexdryden. This sounds like maybe a bug. Are you able to look into it further? It's probably going to be an issue with where the featured items are saved. I think that's an op like saveFeatureOrder or something like that.

[alexdryden]

@NateWr I looked into it a little bit last week. I'm still not super familiar with the architecture, so this may not be the best solution, but it seemed like the best place to patch this would be in the insertFeature() function in the FeatureDAO class here by checking for the MAX value in the seq column and setting $seq to max value + 1.

The function is asking for the $seq value explicitly, but when this is used to simply add another publication to the list of features the default behavior should be to push it to the end of the stack of featured submissions. Right now it is just getting a $seq value of 1 in those cases. Here is my take on a solution in that function.

That solution doesn't break the saveFeaturedOrder() function, which calls insertFeature(), but instead of explicitly getting $seq passed from the form it gets it implicitly from the order of features in in the form. I'll keep looking at it and see if there is a better solution that doesn't rely on implicit ordering.

[NateWr]

Yeah that looks like it would work, but we try to keep that kind of business logic out of the DAOs. Ideally they should just be for reading and writing data, and the logic which determines what the data looks like should be handled elsewhere.

It looks like the problem is in CatalogListItem.vue and it applies to features and new releases. Unfortunately, the proper seq to assign is not available at that level.

I think there are two options here:

1. The code in CatalogListItem.vue that toggles featured and new release assignments and saves that data through the API could be moved to CatalogListPanel.vue. Then CatalogListItem.vue can emit an event that triggers that to happen in CatalogListPanel.vue, where the appropriate seq could be determined and assigned. This strikes me as the most appropriate route, but it requires some familiarity with Vue.js and a fair amount of refactoring on the frontend.

2. Another option would be to explicitly not set a seq in CatalogListItem.vue, or to set it to something like -1. This could then be detected when the request is handled by the API (here). And something like the code you proposed, which got the max sequence and added it in, could be applied before insertFeature() is called.

[alexdryden]

Yeah, something like option 2 was where I was heading. I'll check out option 1 first, if for no other reason then to help familiarize myself with more of the code base, and then fall back to option 2 if it seems like more work than I can fit in this week.