adamsaghy commented on code in PR #5956:
URL: https://github.com/apache/fineract/pull/5956#discussion_r3374514337
##########
fineract-provider/src/main/java/org/apache/fineract/notification/service/NotificationReadPlatformServiceImpl.java:
##########
@@ -92,127 +80,51 @@ private boolean createUpdateCacheValue(Long appUserId,
Long now,
}
private boolean checkForUnreadNotifications(Long appUserId) {
- String sql = "SELECT id, notification_id as notificationId, user_id as
userId, is_read as isRead, created_at "
- + "as createdAt FROM notification_mapper WHERE user_id = ? AND
is_read = false";
- List<NotificationMapperData> notificationMappers =
this.jdbcTemplate.query(sql, notificationMapperRow, appUserId);
- return notificationMappers.size() > 0;
+ return
this.notificationMapperRepository.hasUnreadNotifications(appUserId);
}
@Override
public void updateNotificationReadStatus() {
final Long appUserId = context.authenticatedUser().getId();
- String sql = "UPDATE notification_mapper SET is_read = true WHERE
is_read = false and user_id = ?";
- this.jdbcTemplate.update(sql, appUserId);
+
this.notificationMapperRepository.markUnreadNotificationsAsRead(appUserId);
}
@Override
public Page<NotificationData> getAllUnreadNotifications(final
SearchParameters searchParameters) {
final Long appUserId = context.authenticatedUser().getId();
- String sql = "SELECT " + sqlGenerator.calcFoundRows() + " ng.id as id,
nm.user_id as userId, ng.object_type as objectType, "
- + "ng.object_identifier as objectId, ng.actor as actor, ng." +
sqlGenerator.escape("action")
- + " as action, ng.notification_content "
- + "as content, ng.is_system_generated as isSystemGenerated,
nm.created_at as createdAt "
- + "FROM notification_mapper nm INNER JOIN
notification_generator ng ON nm.notification_id = ng.id "
- + "WHERE nm.user_id = ? AND nm.is_read = false order by
nm.created_at desc";
-
- return getNotificationDataPage(searchParameters, appUserId, sql);
+ final Pageable pageable = toPageable(searchParameters);
+ final org.springframework.data.domain.Page<NotificationData>
springPage = this.notificationMapperRepository
+ .findNotificationDataByUserIdAndReadStatus(appUserId, false,
pageable);
+ return toFineractPage(springPage);
}
@Override
- public Page<NotificationData> getAllNotifications(SearchParameters
searchParameters) {
+ public Page<NotificationData> getAllNotifications(final SearchParameters
searchParameters) {
final Long appUserId = context.authenticatedUser().getId();
- String sql = "SELECT " + sqlGenerator.calcFoundRows() + " ng.id as id,
nm.user_id as userId, ng.object_type as objectType, "
- + "ng.object_identifier as objectId, ng.actor as actor, ng." +
sqlGenerator.escape("action")
- + " as action, ng.notification_content "
- + "as content, ng.is_system_generated as isSystemGenerated,
nm.created_at as createdAt "
- + "FROM notification_mapper nm INNER JOIN
notification_generator ng ON nm.notification_id = ng.id "
- + "WHERE nm.user_id = ? order by nm.created_at desc";
-
- return getNotificationDataPage(searchParameters, appUserId, sql);
+ final Pageable pageable = toPageable(searchParameters);
+ final org.springframework.data.domain.Page<NotificationData>
springPage = this.notificationMapperRepository
+ .findNotificationDataByUserIdAndReadStatus(appUserId, null,
pageable);
+ return toFineractPage(springPage);
}
- private Page<NotificationData> getNotificationDataPage(SearchParameters
searchParameters, Long appUserId, String sql) {
- final StringBuilder sqlBuilder = new StringBuilder(200);
- sqlBuilder.append(sql);
+ private Pageable toPageable(final SearchParameters searchParameters) {
+ final int limit = searchParameters.hasLimit() ?
searchParameters.getLimit() : SearchParameters.DEFAULT_MAX_LIMIT;
+ final int offset = searchParameters.hasOffset() ?
searchParameters.getOffset() : 0;
+ final int page = offset / limit;
Review Comment:
Something like below would be safer here, no?
```
final int limit = searchParameters.hasLimit()
? searchParameters.getLimit()
: SearchParameters.DEFAULT_MAX_LIMIT;
final int offset = searchParameters.hasOffset()
? searchParameters.getOffset()
: 0;
if (limit <= 0) {
throw new IllegalArgumentException("Limit must be greater than zero");
}
if (offset < 0) {
throw new IllegalArgumentException("Offset must not be negative");
}
final int page = offset / limit;
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
