Added a filter function to 'browse publications' which filters for primary files. #254
Conversation
leenapthine
commented
Nov 28, 2024
leenapthine
commented
- Added a filter box to the 'browse publication' search filter that filters out publications that do not contain a primary file.
- Added the logic for the filter
- Styled the checkbox slightly (this will probably want more input from Graham).
|
Sorry, @pauravhp, quick wording request (before we move out of the review phase): could we please change the wording to "Only show publications with attachments"? I wonder if that might be easier for most people to understand than the current wording. |
dleske
left a comment
dleske
left a comment
There was a problem hiding this comment.
Looks okay to me. I note that <? $x ?> can be used instead of <?php echo $x ?> but if the full syntax is what's used in the codebase then we should stick with that.
| // Run query with limit | ||
| $results = $model->entries('list', $filters); | ||
|
|
||
| // Lee: Check that publication has an attachment |
There was a problem hiding this comment.
Is prefixing a comment with the developer's name typical for this codebase?
If we're tagging this code as local modifications, then I would prefer "UVic" or "ARCsoft" as appropriate--git blame can always be used to determine the individual developer.
If this is a reminder to check something again, TODO or TODO(drew): is the way I would go.
There was a problem hiding this comment.
I've run across some of these comments when browsing through the codebase and it seems like these are just for features. Moving forward if I run across them I will modify them to say ARCsoft or TODO depending on the situation (although many of them are not TODOs I believe). Do you need me to change the highlighted line to ARCsoft?
There was a problem hiding this comment.
I don't see any reason to have that or Lee: in this line at all. Please take it out or clarify.
There was a problem hiding this comment.
Moving forward TODO is appropriate. Actually I should have asked if @ghjensen requested we do this to mark off sections we've modified--is this the case?
If not, I'd say we use TODO if it's a TODO, and otherwise leave out these labels unless/until we have a reason to use them.
|
Looks good to me |
