Closes #2509: New notifications UI #2530
Conversation
| static public UserNotificationQuery newQuery() { | ||
| return new UserNotificationQuery(); | ||
| } |
There was a problem hiding this comment.
If we are making this a builder kind of class then I think that we should block creating the object outside of it by adding private constructor.
There was a problem hiding this comment.
Yeah, good point, added private constructor.
| int resultLimit = DEFAULT_RESULT_LIMIT; | ||
|
|
||
| boolean ascending; |
There was a problem hiding this comment.
Any reason why this is not private? Do we need it to be package private?
|
|
||
| List<Predicate> predicates = new ArrayList<>(); | ||
| if (StringUtils.isNotBlank(queryParam.getSearchLabel())) { | ||
| predicates.add(criteriaBuilder.like(criteriaBuilder.lower(root.get("searchLabel")), "%" + queryParam.getSearchLabel().toLowerCase() + "%")); |
There was a problem hiding this comment.
Do index on searchLabel will be used when we lower case it in the query? If yes then ok. If not then I would consider to keep lowercased searchLabel in the DB (since we use this field only for searching) and skip lowercasing it here.
There was a problem hiding this comment.
Setting it to lower case in the db seems actually cleaner to me. I've adjusted that.
| notification.deleteSelected.btn=Usu\u0144 wybrane | ||
| notification.deleteSelected=Usu\u0144 powiadomienia | ||
| notification.deleteSelected.info=Powiadomienia zostan\u0105 usuni\u0119ty po naci\u015Bni\u0119ciu przycisku "Usu\u0144". | ||
| notification.numSelected=Obecnie wybrano {0} {0,choice,0#powiadomie\u0144|1#powiadomienie|2#powiadomie\u0144} |
There was a problem hiding this comment.
Depending on the number we will have:
Obecnie wybrano 0 powiadomień- OKObecnie wybrano 1 powiadomienie- OKObecnie wybrano 2 powiadomień- I think it should bepowiadomieniaObecnie wybrano 3 powiadomień- As aboveObecnie wybrano 4 powiadomień- As aboveObecnie wybrano 5 powiadomień- OK
| @GET | ||
| @Path("usernotifications") |
There was a problem hiding this comment.
I assume that method GET is to keep the consistency between other endpoints in Index. Is that right? Because endpoint will result in some change in DB. If yes then then ignore this comment
There was a problem hiding this comment.
Yeah, it's to be consistent with the other endpoints. They're all GET requests despite performing actions/write operations (apart from 2 DELETE which accurately delete things). Didn't want to break that "consistency" for this new one.
New UI to manage notifications:
searchLabelcolumn. The column is filled when notification is added based on the object pointed by theobjectid.adminAPI (/admin/index/usernotifications) to fill out the new column for existing notificationsAlternatively to the DB based filtering we could add a new solr index.
Issue: #2509