-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add categories to knowledge center #687
base: main
Are you sure you want to change the base?
Conversation
@jtucholski we are thinking about implementing multi select for this query. do you have any oppositions to that? |
@calisio That's fine. If the user selects all of the options can it switch to "All Libraries"? |
provider-middleware/kiwix.go
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want to test this locally, uncomment this code to seed random categories into your db. For the real version, we will have these categories seeded into our databases directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will drop this component when Rich's PR gets merged in (it is pulled from his code but we just needed it to finish our ui)
@PThorpe92 should be updated now, but will need to modify again once rich's search is merged |
@@ -59,6 +59,9 @@ func (db *DB) GetAllLibraries(page, perPage, days int, userId, facilityId uint, | |||
search = "%" + strings.ToLower(search) + "%" | |||
tx = tx.Where("LOWER(libraries.title) LIKE ? OR LOWER(libraries.description) LIKE ?", search, search) | |||
} | |||
if len(categoryIds) > 0 { | |||
tx = tx.Select("DISTINCT libraries.*").Joins("JOIN open_content_types t ON t.content_id = libraries.id").Where("t.category_id IN (?)", categoryIds) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should have the following criteria added
t.open_content_provider_id = libraries.open_content_provider_id
otherwise you will pull all other providers' open_content_types records if there are any.
@@ -59,6 +59,9 @@ func (db *DB) GetAllLibraries(page, perPage, days int, userId, facilityId uint, | |||
search = "%" + strings.ToLower(search) + "%" | |||
tx = tx.Where("LOWER(libraries.title) LIKE ? OR LOWER(libraries.description) LIKE ?", search, search) | |||
} | |||
if len(categoryIds) > 0 { | |||
tx = tx.Select("DISTINCT libraries.*").Joins("JOIN open_content_types t ON t.content_id = libraries.id").Where("t.category_id IN (?)", categoryIds) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious why you used DISTINCT here? I think you can remove this keyword if you make join on open_content_provider_id.
func (db *DB) GetCategories() ([]models.OpenContentCategory, error) { | ||
var categories []models.OpenContentCategory | ||
if err := db.Model(&models.OpenContentCategory{}).Find(&categories).Error; err != nil { | ||
return nil, newNotFoundDBError(err, "categories") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change "categories" to the table name "open_content_categories"
@@ -182,6 +184,7 @@ const LibrarySearchResultsModal = forwardRef< | |||
onModalClose(); | |||
setSearchResults(BlankChannel); | |||
}; | |||
console.log(libraryOptions); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can this be removed?
@@ -234,7 +234,6 @@ const router = createBrowserRouter([ | |||
path: 'viewer/libraries/:id', | |||
element: <LibraryViewer />, | |||
errorElement: <Error />, | |||
loader: getLibraryOptions, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
loader is needed here for loading library options
@@ -43,7 +52,7 @@ export const getStudentLevel1Data: LoaderFunction = async () => { | |||
helpfulLinks: helpfulLinks, | |||
topUserContent: topUserOpenContent, | |||
topFacilityContent: topFacilityOpenContent, | |||
favorites: favoriteOpenContent, | |||
favorites: favoriteOpenContent, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you may need to add the logic for building the list of options here like you did in getLibraryLayoutData, these options will be used on the trending content page.
Description of the change
Adds category dropdown to knowledge center for students and admins.
Screenshot(s)
Additional context
Leaving this as a draft PR for now, we will be working on implementing AI to sort these features.
Proposed enhancement
We are also playing around with the idea of selecting more than one option from the dropdown.