You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by GitBox <gi...@apache.org> on 2022/05/17 20:11:03 UTC
[GitHub] [fineract] galovics opened a new pull request, #2330: [DO NOT MERGE] Topics cleanup
galovics opened a new pull request, #2330:
URL: https://github.com/apache/fineract/pull/2330
## Description
Describe the changes made and why they were made.
Ignore if these details are present on the associated [Apache Fineract JIRA ticket](https://github.com/apache/fineract/pull/1284).
## Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
- [ ] Write the commit message as per https://github.com/apache/fineract/#pull-requests
- [ ] Acknowledge that we will not review PRs that are not passing the build _("green")_ - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.
- [ ] Create/update unit or integration tests for verifying the changes made.
- [ ] Follow coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions.
- [ ] Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes
- [ ] Submission is not a "code dump". (Large changes can be made "in repository" via a branch. Ask on the developer mailing list for guidance, if required.)
FYI our guidelines for code reviews are at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide.
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] galovics commented on a diff in pull request #2330: Topics cleanup
Posted by GitBox <gi...@apache.org>.
galovics commented on code in PR #2330:
URL: https://github.com/apache/fineract/pull/2330#discussion_r876999955
##########
fineract-provider/src/main/java/org/apache/fineract/notification/service/NotificationDomainServiceImpl.java:
##########
@@ -387,39 +379,15 @@ private void buildNotification(String permission, String objectType, Long object
String eventType, Long appUserId, Long officeId) {
String tenantIdentifier = ThreadLocalContextUtil.getTenant().getTenantIdentifier();
- Queue queue = new ActiveMQQueue("NotificationQueue");
- List<Long> userIds = retrieveSubscribers(officeId, permission);
+ Set<Long> userIds = getNotifiableUserIds(officeId, permission);
NotificationData notificationData = new NotificationData(objectType, objectIdentifier, eventType, appUserId, notificationContent,
false, false, tenantIdentifier, officeId, userIds);
- try {
- this.notificationEvent.broadcastNotification(queue, notificationData);
- } catch (Exception e) {
- this.springEventPublisher.broadcastNotification(notificationData);
- }
+ notificationEventPublisher.broadcastNotification(notificationData);
Review Comment:
Same as above mostly. However the transactional context is a good thing you pointed out.
The original implementation also suffers from this in case the fallback logic fails somewhere - which frankly is a possibility.
I'll add a simple try catch here to handle potential errors and prevent them from rolling back the tx. Thanks.
##########
fineract-provider/src/main/java/org/apache/fineract/notification/service/NotificationDomainServiceImpl.java:
##########
@@ -387,39 +379,15 @@ private void buildNotification(String permission, String objectType, Long object
String eventType, Long appUserId, Long officeId) {
String tenantIdentifier = ThreadLocalContextUtil.getTenant().getTenantIdentifier();
- Queue queue = new ActiveMQQueue("NotificationQueue");
- List<Long> userIds = retrieveSubscribers(officeId, permission);
+ Set<Long> userIds = getNotifiableUserIds(officeId, permission);
NotificationData notificationData = new NotificationData(objectType, objectIdentifier, eventType, appUserId, notificationContent,
false, false, tenantIdentifier, officeId, userIds);
- try {
- this.notificationEvent.broadcastNotification(queue, notificationData);
- } catch (Exception e) {
- this.springEventPublisher.broadcastNotification(notificationData);
- }
+ notificationEventPublisher.broadcastNotification(notificationData);
}
- private List<Long> retrieveSubscribers(Long officeId, String permission) {
-
- Set<TopicSubscriberData> topicSubscribers = new HashSet<>();
- List<Long> subscriberIds = new ArrayList<>();
- Long entityId = officeId;
- String entityType = "";
- if (officeRepository.findById(entityId).get().getParent() == null) {
- entityType = "OFFICE";
- } else {
- entityType = "BRANCH";
- }
- List<Role> allRoles = roleRepository.findAll();
- for (Role curRole : allRoles) {
- if (curRole.hasPermissionTo(permission) || curRole.hasPermissionTo("ALL_FUNCTIONS")) {
- String memberType = curRole.getName();
- topicSubscribers.addAll(topicSubscriberReadPlatformService.getSubscribers(entityId, entityType, memberType));
- }
- }
-
- for (TopicSubscriberData topicSubscriber : topicSubscribers) {
- subscriberIds.add(topicSubscriber.getUserId());
- }
- return subscriberIds;
+ private Set<Long> getNotifiableUserIds(Long officeId, String permission) {
+ Collection<AppUser> users = appUserRepository.findByOfficeId(officeId);
+ Collection<AppUser> usersWithPermission = users.stream().filter(aU -> aU.hasAnyPermission(permission, "ALL_FUNCTIONS")).toList();
+ return usersWithPermission.stream().map(AppUser::getId).collect(toSet());
Review Comment:
Tried my best. :)
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] galovics commented on a diff in pull request #2330: Topics cleanup
Posted by GitBox <gi...@apache.org>.
galovics commented on code in PR #2330:
URL: https://github.com/apache/fineract/pull/2330#discussion_r877003153
##########
fineract-provider/src/main/java/org/apache/fineract/notification/service/NotificationWritePlatformServiceImpl.java:
##########
@@ -64,7 +53,7 @@ public Long notify(Long userId, String objectType, Long objectIdentifier, String
private Long insertIntoNotificationMapper(Long userId, Long generatedNotificationId) {
AppUser appUser = this.appUserRepository.findById(userId).orElse(null);
NotificationMapper notificationMapper = new NotificationMapper(
- this.notificationGeneratorReadRepositoryWrapper.findById(generatedNotificationId), appUser, false, getCurrentDateTime());
+ this.notificationGeneratorReadRepositoryWrapper.findById(generatedNotificationId), appUser, false, new Date());
Review Comment:
Good point. I replicated the original implementation with the new Date but I'm gonna switch it to the tenant date. Thanks.
##########
fineract-provider/src/main/java/org/apache/fineract/notification/service/NotificationWritePlatformServiceImpl.java:
##########
@@ -74,13 +63,13 @@ private Long insertIntoNotificationGenerator(String objectType, Long objectIdent
String notificationContent, boolean isSystemGenerated) {
Notification notification = new Notification(objectType, objectIdentifier, action, actorId, isSystemGenerated, notificationContent,
- getCurrentDateTime());
+ new Date());
Review Comment:
Same as above.
##########
fineract-provider/src/main/java/org/apache/fineract/notification/service/NotificationWritePlatformServiceImpl.java:
##########
@@ -90,22 +79,15 @@ public Long notify(List<Long> userIds, String objectType, Long objectId, String
return generatedNotificationId;
}
- private List<Long> insertIntoNotificationMapper(List<Long> userIds, Long generatedNotificationId) {
+ private List<Long> insertIntoNotificationMapper(Collection<Long> userIds, Long generatedNotificationId) {
List<Long> mappedIds = new ArrayList<>();
for (Long userId : userIds) {
AppUser appUser = this.appUserRepository.findById(userId).get();
NotificationMapper notificationMapper = new NotificationMapper(
- this.notificationGeneratorReadRepositoryWrapper.findById(generatedNotificationId), appUser, false,
- getCurrentDateTime());
+ this.notificationGeneratorReadRepositoryWrapper.findById(generatedNotificationId), appUser, false, new Date());
Review Comment:
Same as above.
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2330: Topics cleanup
Posted by GitBox <gi...@apache.org>.
adamsaghy commented on code in PR #2330:
URL: https://github.com/apache/fineract/pull/2330#discussion_r877005486
##########
fineract-provider/src/main/java/org/apache/fineract/notification/service/NotificationWritePlatformServiceImpl.java:
##########
@@ -64,7 +53,7 @@ public Long notify(Long userId, String objectType, Long objectIdentifier, String
private Long insertIntoNotificationMapper(Long userId, Long generatedNotificationId) {
AppUser appUser = this.appUserRepository.findById(userId).orElse(null);
NotificationMapper notificationMapper = new NotificationMapper(
- this.notificationGeneratorReadRepositoryWrapper.findById(generatedNotificationId), appUser, false, getCurrentDateTime());
+ this.notificationGeneratorReadRepositoryWrapper.findById(generatedNotificationId), appUser, false, new Date());
Review Comment:
Changed my mind. you can leave it as is for now.. there are already a bunch of places where the created_date system date and some places where it is tenant date. We shall figure those out later in a generic, common way
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] galovics merged pull request #2330: Topics cleanup
Posted by GitBox <gi...@apache.org>.
galovics merged PR #2330:
URL: https://github.com/apache/fineract/pull/2330
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] galovics commented on a diff in pull request #2330: Topics cleanup
Posted by GitBox <gi...@apache.org>.
galovics commented on code in PR #2330:
URL: https://github.com/apache/fineract/pull/2330#discussion_r876998243
##########
fineract-provider/src/main/java/org/apache/fineract/notification/eventandlistener/ActiveMQNotificationEventPublisher.java:
##########
@@ -18,33 +18,24 @@
*/
package org.apache.fineract.notification.eventandlistener;
-import javax.jms.Destination;
-import javax.jms.JMSException;
-import javax.jms.Message;
-import javax.jms.Session;
+import javax.jms.Queue;
+import lombok.RequiredArgsConstructor;
+import org.apache.activemq.command.ActiveMQQueue;
import org.apache.fineract.notification.data.NotificationData;
-import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.context.annotation.Profile;
import org.springframework.jms.core.JmsTemplate;
-import org.springframework.jms.core.MessageCreator;
import org.springframework.stereotype.Service;
@Service
-public class NotificationEventService {
+@Profile("activeMqEnabled")
+@RequiredArgsConstructor
+public class ActiveMQNotificationEventPublisher implements NotificationEventPublisher {
private final JmsTemplate jmsTemplate;
- @Autowired
- public NotificationEventService(JmsTemplate jmsTemplate) {
- this.jmsTemplate = jmsTemplate;
- }
-
- public void broadcastNotification(final Destination destination, final NotificationData notificationData) {
- this.jmsTemplate.send(destination, new MessageCreator() {
-
- @Override
- public Message createMessage(Session session) throws JMSException {
- return session.createObjectMessage(notificationData);
- }
- });
+ @Override
+ public void broadcastNotification(NotificationData notificationData) {
+ Queue queue = new ActiveMQQueue("NotificationQueue");
Review Comment:
You're not wrong there but I intentionally left it here since this was the original implementation and I didn't want to spend more time on testing the ActiveMQ piece as well.
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2330: Topics cleanup
Posted by GitBox <gi...@apache.org>.
adamsaghy commented on code in PR #2330:
URL: https://github.com/apache/fineract/pull/2330#discussion_r876374527
##########
fineract-provider/src/main/java/org/apache/fineract/notification/eventandlistener/SpringNotificationEventListener.java:
##########
@@ -0,0 +1,42 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.notification.eventandlistener;
+
+import lombok.RequiredArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.fineract.notification.data.NotificationData;
+import org.springframework.context.ApplicationListener;
+import org.springframework.context.annotation.Profile;
+import org.springframework.stereotype.Component;
+
+@Component
+@Profile("!activeMqEnabled")
Review Comment:
Even the the activeMq is enabled, still it might not working as expected and as a fallback logic we still need to publish and receive notification events. My recommendation is to remove the profile.
##########
fineract-provider/src/main/java/org/apache/fineract/notification/eventandlistener/ActiveMQNotificationEventPublisher.java:
##########
@@ -18,33 +18,24 @@
*/
package org.apache.fineract.notification.eventandlistener;
-import javax.jms.Destination;
-import javax.jms.JMSException;
-import javax.jms.Message;
-import javax.jms.Session;
+import javax.jms.Queue;
+import lombok.RequiredArgsConstructor;
+import org.apache.activemq.command.ActiveMQQueue;
import org.apache.fineract.notification.data.NotificationData;
-import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.context.annotation.Profile;
import org.springframework.jms.core.JmsTemplate;
-import org.springframework.jms.core.MessageCreator;
import org.springframework.stereotype.Service;
@Service
-public class NotificationEventService {
+@Profile("activeMqEnabled")
+@RequiredArgsConstructor
+public class ActiveMQNotificationEventPublisher implements NotificationEventPublisher {
private final JmsTemplate jmsTemplate;
- @Autowired
- public NotificationEventService(JmsTemplate jmsTemplate) {
- this.jmsTemplate = jmsTemplate;
- }
-
- public void broadcastNotification(final Destination destination, final NotificationData notificationData) {
- this.jmsTemplate.send(destination, new MessageCreator() {
-
- @Override
- public Message createMessage(Session session) throws JMSException {
- return session.createObjectMessage(notificationData);
- }
- });
+ @Override
+ public void broadcastNotification(NotificationData notificationData) {
+ Queue queue = new ActiveMQQueue("NotificationQueue");
Review Comment:
It might have been moved into configuration.
##########
fineract-provider/src/main/java/org/apache/fineract/notification/service/NotificationDomainServiceImpl.java:
##########
@@ -387,39 +379,15 @@ private void buildNotification(String permission, String objectType, Long object
String eventType, Long appUserId, Long officeId) {
String tenantIdentifier = ThreadLocalContextUtil.getTenant().getTenantIdentifier();
- Queue queue = new ActiveMQQueue("NotificationQueue");
- List<Long> userIds = retrieveSubscribers(officeId, permission);
+ Set<Long> userIds = getNotifiableUserIds(officeId, permission);
NotificationData notificationData = new NotificationData(objectType, objectIdentifier, eventType, appUserId, notificationContent,
false, false, tenantIdentifier, officeId, userIds);
- try {
- this.notificationEvent.broadcastNotification(queue, notificationData);
- } catch (Exception e) {
- this.springEventPublisher.broadcastNotification(notificationData);
- }
+ notificationEventPublisher.broadcastNotification(notificationData);
}
- private List<Long> retrieveSubscribers(Long officeId, String permission) {
-
- Set<TopicSubscriberData> topicSubscribers = new HashSet<>();
- List<Long> subscriberIds = new ArrayList<>();
- Long entityId = officeId;
- String entityType = "";
- if (officeRepository.findById(entityId).get().getParent() == null) {
- entityType = "OFFICE";
- } else {
- entityType = "BRANCH";
- }
- List<Role> allRoles = roleRepository.findAll();
- for (Role curRole : allRoles) {
- if (curRole.hasPermissionTo(permission) || curRole.hasPermissionTo("ALL_FUNCTIONS")) {
- String memberType = curRole.getName();
- topicSubscribers.addAll(topicSubscriberReadPlatformService.getSubscribers(entityId, entityType, memberType));
- }
- }
-
- for (TopicSubscriberData topicSubscriber : topicSubscribers) {
- subscriberIds.add(topicSubscriber.getUserId());
- }
- return subscriberIds;
+ private Set<Long> getNotifiableUserIds(Long officeId, String permission) {
+ Collection<AppUser> users = appUserRepository.findByOfficeId(officeId);
+ Collection<AppUser> usersWithPermission = users.stream().filter(aU -> aU.hasAnyPermission(permission, "ALL_FUNCTIONS")).toList();
+ return usersWithPermission.stream().map(AppUser::getId).collect(toSet());
Review Comment:
I have to trust you on this...
##########
fineract-provider/src/main/java/org/apache/fineract/notification/service/NotificationWritePlatformServiceImpl.java:
##########
@@ -64,7 +53,7 @@ public Long notify(Long userId, String objectType, Long objectIdentifier, String
private Long insertIntoNotificationMapper(Long userId, Long generatedNotificationId) {
AppUser appUser = this.appUserRepository.findById(userId).orElse(null);
NotificationMapper notificationMapper = new NotificationMapper(
- this.notificationGeneratorReadRepositoryWrapper.findById(generatedNotificationId), appUser, false, getCurrentDateTime());
+ this.notificationGeneratorReadRepositoryWrapper.findById(generatedNotificationId), appUser, false, new Date());
Review Comment:
Please rather use tenant date...
##########
fineract-provider/src/main/java/org/apache/fineract/notification/eventandlistener/SpringNotificationEventPublisher.java:
##########
@@ -18,18 +18,24 @@
*/
package org.apache.fineract.notification.eventandlistener;
+import lombok.RequiredArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
import org.apache.fineract.notification.data.NotificationData;
-import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.ApplicationEventPublisher;
+import org.springframework.context.annotation.Profile;
import org.springframework.stereotype.Service;
@Service
-public class SpringEventPublisher {
+@Profile("!activeMqEnabled")
Review Comment:
Same as above
##########
fineract-provider/src/main/java/org/apache/fineract/notification/service/NotificationDomainServiceImpl.java:
##########
@@ -387,39 +379,15 @@ private void buildNotification(String permission, String objectType, Long object
String eventType, Long appUserId, Long officeId) {
String tenantIdentifier = ThreadLocalContextUtil.getTenant().getTenantIdentifier();
- Queue queue = new ActiveMQQueue("NotificationQueue");
- List<Long> userIds = retrieveSubscribers(officeId, permission);
+ Set<Long> userIds = getNotifiableUserIds(officeId, permission);
NotificationData notificationData = new NotificationData(objectType, objectIdentifier, eventType, appUserId, notificationContent,
false, false, tenantIdentifier, officeId, userIds);
- try {
- this.notificationEvent.broadcastNotification(queue, notificationData);
- } catch (Exception e) {
- this.springEventPublisher.broadcastNotification(notificationData);
- }
+ notificationEventPublisher.broadcastNotification(notificationData);
Review Comment:
My concern: the notification publishing logic got kind of different.
Priorly: it was sending over JMS and if it failed for any reason we had a fallback logic to send it over internal spring event.
Now: the publisher is decided based on profile, but this way we dont have fallback logic anymore, also the error can bubble up and might triggering a rollback which is an unwanted behaviour.
##########
fineract-provider/src/main/java/org/apache/fineract/notification/service/NotificationWritePlatformServiceImpl.java:
##########
@@ -90,22 +79,15 @@ public Long notify(List<Long> userIds, String objectType, Long objectId, String
return generatedNotificationId;
}
- private List<Long> insertIntoNotificationMapper(List<Long> userIds, Long generatedNotificationId) {
+ private List<Long> insertIntoNotificationMapper(Collection<Long> userIds, Long generatedNotificationId) {
List<Long> mappedIds = new ArrayList<>();
for (Long userId : userIds) {
AppUser appUser = this.appUserRepository.findById(userId).get();
NotificationMapper notificationMapper = new NotificationMapper(
- this.notificationGeneratorReadRepositoryWrapper.findById(generatedNotificationId), appUser, false,
- getCurrentDateTime());
+ this.notificationGeneratorReadRepositoryWrapper.findById(generatedNotificationId), appUser, false, new Date());
Review Comment:
Please rather use tenant date...
##########
fineract-provider/src/main/java/org/apache/fineract/notification/service/NotificationWritePlatformServiceImpl.java:
##########
@@ -74,13 +63,13 @@ private Long insertIntoNotificationGenerator(String objectType, Long objectIdent
String notificationContent, boolean isSystemGenerated) {
Notification notification = new Notification(objectType, objectIdentifier, action, actorId, isSystemGenerated, notificationContent,
- getCurrentDateTime());
+ new Date());
Review Comment:
Please rather use tenant date...
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] galovics commented on a diff in pull request #2330: Topics cleanup
Posted by GitBox <gi...@apache.org>.
galovics commented on code in PR #2330:
URL: https://github.com/apache/fineract/pull/2330#discussion_r876999734
##########
fineract-provider/src/main/java/org/apache/fineract/notification/eventandlistener/SpringNotificationEventListener.java:
##########
@@ -0,0 +1,42 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.notification.eventandlistener;
+
+import lombok.RequiredArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.fineract.notification.data.NotificationData;
+import org.springframework.context.ApplicationListener;
+import org.springframework.context.annotation.Profile;
+import org.springframework.stereotype.Component;
+
+@Component
+@Profile("!activeMqEnabled")
Review Comment:
Right. I was struggling between 2 ideas:
- Leave the existing approach i.e. the fallback
- Skip the fallback logic completely
I decided on the second one because I felt like with the fallback we're kind of hiding the fact if there's a problem with the ActiveMQ broker. I'd say let's fail-fast. If there's an issue with the broker, make sure it shows up everywhere instead of the fallback handling the case.
##########
fineract-provider/src/main/java/org/apache/fineract/notification/service/NotificationDomainServiceImpl.java:
##########
@@ -387,39 +379,15 @@ private void buildNotification(String permission, String objectType, Long object
String eventType, Long appUserId, Long officeId) {
String tenantIdentifier = ThreadLocalContextUtil.getTenant().getTenantIdentifier();
- Queue queue = new ActiveMQQueue("NotificationQueue");
- List<Long> userIds = retrieveSubscribers(officeId, permission);
+ Set<Long> userIds = getNotifiableUserIds(officeId, permission);
NotificationData notificationData = new NotificationData(objectType, objectIdentifier, eventType, appUserId, notificationContent,
false, false, tenantIdentifier, officeId, userIds);
- try {
- this.notificationEvent.broadcastNotification(queue, notificationData);
- } catch (Exception e) {
- this.springEventPublisher.broadcastNotification(notificationData);
- }
+ notificationEventPublisher.broadcastNotification(notificationData);
Review Comment:
Same as above.
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org