project-lux / lux-marklogic

Code, issues, and resources related to LUX MarkLogic
Other
3 stars 2 forks source link

Add index properties for Public Domain AS boolean option #103

Closed roamye closed 2 weeks ago

roamye commented 2 months ago

Problem Description: The issue: https://github.com/project-lux/lux-frontend/issues/47 would like to create a new advanced search Boolean "Are in the Public Doman". This ML issue supports FE47 by looking for a boolean value represented by a 1 or 0 in the indexed properties

Expected Behavior/Solution:

Example | Are in the Public Domain: Yes (A Noble Lord):

e.g. the isPublicDomain below

"indexedProperties": {
"dataType": "VisualItem", 
"uiType": "CollectionWork", 
"isCollectionItem": 0, 
"hasDigitalImage": 0, 
"isOnline": 0, 
"isPublicDomain": 1
}

Requirements:

Needed for promotion: If an item on the list is not needed, it should be crossed off but not removed.

UAT/LUX Examples:

Dependencies/Blocks:

Related Github Issues:

Related links:

Wireframe/Mockup: ~Place wireframe/mockup for the proposed solution at end of ticket.~

roamye commented 2 months ago

@clarkepeterf I created this ML ticket for https://github.com/project-lux/lux-frontend/issues/47. Feel free to update the title/body.

prowns commented 2 months ago

Added to next Ticket forming - this is blocking an important FE ticket. @clarkepeterf - what is needed to finish forming this ticket?

clarkepeterf commented 2 months ago

@azaroth42 Based on findings from adding itemHasDigitalImage in this ticket from the old repo - We need to add a property to the indexedProperties section in the data.

If we try to use the data as-is, Works which don't have a subject_to field will not appear in the facet (e.g. we could have a 'Yes' for public domain, but we wouldn't have a 'No' for public domain)

If we want to be able to check either 'Yes' or 'No' for being public domain, we need a separate indexed property with a boolean value represented by a 1 or a 0

clarkepeterf commented 1 month ago

@azaroth42 Regarding the comment above, can you confirm if it will be possible to add a workIsInPublicDomain field to the indexedProperties?

clarkepeterf commented 1 month ago

@azaroth42 If a facet is not needed (just advanced search) it could also be implemented using the existing JSON, not as a boolean, but as a 'Yes'-matching-only field. (It could still be surrounded by a NOT query) - I suppose that means it could also be turned into a boolean field with some extra tweaking on the frontend. Let me know what is desired here

azaroth42 commented 1 month ago

I can add it to indexedProperties. Will make a data-pipeline ticket to block this one.

azaroth42 commented 1 month ago

https://github.com/project-lux/data-pipeline/issues/65

I've added isPublicDomain on works, default to 0, and then reset to 1 if the CC0 URI is in subject_to. Will be in next dataset build.

roamye commented 1 month ago

@clarkepeterf once this has been updated please leave a comment.

roamye commented 1 month ago

The issue as been re-scoped. @azaroth42 can you please review and verify if this issue is ready for backlog?

Thank you.

cc: @prowns @jffcamp

azaroth42 commented 1 month ago

looks good!

clarkepeterf commented 1 month ago

@kamerynB the ML part is deployed to DEV, ready for the frontend. Also, the help text has newlines, but they don't show up on the frontend. Could you show newlines on the frontend? I think there are multiple ways to allow newlines on the frontend, one of which being adding the white-space: pre-wrap property to the CSS for this element.

kamerynB commented 1 month ago

@clarkepeterf Yes, the frontend has multiple ways of adding new lines but it depends on if you want the line break to be at specific spot. If you know where the line break should occur, I believe inserting a <br /> tag should work when the frontend reads it.

prowns commented 1 month ago

@clarkepeterf and @kamerynB - it's ok if this text ends up as a single paragraph as well.

clarkepeterf commented 1 month ago

@kamerynB Since it is interpreted from a string <br /> is not processed as HTML, it shows up as a normal string. See DEV right now for an example of including <br /> in the advanced search config string.I think you'd have to use dangerouslySetInnerHTML to allow <br /> to show up.

clarkepeterf commented 1 month ago

@prowns @kamerynB ok, I guess we don't need the spacing in the paragraph. No worries about <br /> or newlines

clarkepeterf commented 1 month ago

@kamerynB although if you do make it use dangerouslySetInnerHtml we could also show a link in the help text, which is what was originally requested. Not necessary though.

kamerynB commented 1 month ago

@clarkepeterf That all sounds good.

prowns commented 1 month ago

@roamye - I'll update the front-end ticket to reflect these changes to the help text.

roamye commented 1 month ago

Approved by UAT

roamye commented 2 weeks ago

Closing as this ticket was marked as Done the week of 6/3.