Skip to content
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

gh-371 Add filtering to get classifier element API call #374

Merged
merged 5 commits into from
Dec 5, 2022

Conversation

GBishop-PHSA
Copy link
Contributor

@GBishop-PHSA GBishop-PHSA commented Nov 25, 2022

gh-371 The get classifier elements endpoint was updated to include filtering by label, description and domain type

Copy link
Contributor

@joe-crawford joe-crawford left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Gail, thanks for adding this.

There are a few things I've noticed:

  • I'm getting a 500 error when I've tried to search by description over classified items. I think the issue is that the classified items have a null description field:
GET http://localhost:8080/api/classifiers/474e5961-456e-4382-b71c-390bbf7b79af/catalogueItems?label=Complex&domainType=data&description=test

{
    "status": 500,
    "reason": "Internal Server Error",
    "errorCode": "UEX--",
    "message": "Cannot invoke method contains() on null object",
    "path": "/api/classifiers/474e5961-456e-4382-b71c-390bbf7b79af/catalogueItems",
    "environment": "DEVELOPMENT",
    "version": "5.3.0-SNAPSHOT",
    "exception": {
        "type": "NullPointerException",
        "message": "Cannot invoke method contains() on null object",
        "stacktrace": [
            "uk.ac.ox.softeng.maurodatamapper.core.container.ClassifierService$__tt__findAllReadableCatalogueItemsByClassifierId_closure55.doCall(ClassifierService.groovy:326)",
            "jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)",
            "uk.ac.ox.softeng.maurodatamapper.core.container.ClassifierService.$tt__findAllReadableCatalogueItemsByClassifierId(ClassifierService.groovy:335)",
            "jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)",
            "jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)",
            "grails.gorm.transactions.GrailsTransactionTemplate$2.doInTransaction(GrailsTransactionTemplate.groovy:94)",
            "org.springframework.transaction.support.TransactionTemplate.execute(TransactionTemplate.java:140)",
            "grails.gorm.transactions.GrailsTransactionTemplate.execute(GrailsTransactionTemplate.groovy:91)",
            "uk.ac.ox.softeng.maurodatamapper.core.container.ClassifierController.catalogueItems(ClassifierController.groovy:41)",
            "jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)",
            "org.grails.core.DefaultGrailsControllerClass$ReflectionInvoker.invoke(DefaultGrailsControllerClass.java:211)",
            "org.grails.core.DefaultGrailsControllerClass.invoke(DefaultGrailsControllerClass.java:188)",
            "org.grails.web.mapping.mvc.UrlMappingsInfoHandlerAdapter.handle(UrlMappingsInfoHandlerAdapter.groovy:90)",
            "org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:1067)",
            "org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:963)",
            "org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:1006)",
            "org.springframework.web.servlet.FrameworkServlet.doGet(FrameworkServlet.java:898)",
            "org.springframework.web.servlet.FrameworkServlet.service(FrameworkServlet.java:883)",
            "org.grails.web.servlet.mvc.GrailsWebRequestFilter.doFilterInternal(GrailsWebRequestFilter.java:77)",
            "org.grails.web.filters.HiddenHttpMethodFilter.doFilterInternal(HiddenHttpMethodFilter.java:67)",
            "java.lang.Thread.run(Thread.java:833)"
        ]
    }
}
  • In similar filters (e.g. the filtering on Data Classes/Elements), we are filtering by an and on the filter fields instead of an or as in this change. I think it would be better if this also filtered on and to be consistent unless there's a specific reason not to?
  • Also in the similar filters above the search is case insensitive, I think this is because of the lowers in CatalogueItemService.applyHQLFilters. Would you be able to make this case insensitive as well?
  • On the Data Model Data Types filter the 'Type' column filter has a select box because the types are in a limited list. I think that would be ideal here as well although it's probably not essential.

Could you have a look at the above?

@joe-crawford
Copy link
Contributor

joe-crawford commented Dec 1, 2022

  • On the Data Model Data Types filter the 'Type' column filter has a select box because the types are in a limited list. I think that would be ideal here as well although it's probably not essential.

I've realised this comment is about the UI of course.

Edit: have commented on UI PR about leaving this due to existing issue with the dropdown filters.

@joe-crawford
Copy link
Contributor

Great, thanks!

@joe-crawford joe-crawford merged commit 6eb96cf into MauroDataMapper:develop Dec 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants