Hi Pranjal,

Thanks for the quick reply! Yes, you have answered my question! 


Thanks,
Adam

> On 19 May 2022, at 18:14, Pranjal Goswami <[email protected]> wrote:
> 
> Hi Arnold, 
> 
> I agree with you. It was over-engineering from the design perspective. The 
> initial discussions in 2016 indicated such scenarios might come up. However, 
> in hindsight it might seem unnecessary. We saw a lot of utility in our own 
> application and hence included it in this project. 
> 
> Hi Adam, 
> 
> Good to meet you too!
> ActiveMQ was introduced majorly for two reasons.
>  
> 1. Make the notification processing (generating message, recipients and 
> actually sending it) asynchronous and detached from the Notification 
> generation process. 
> Keeping it synchronous would mean that the notification generation and 
> delivery procedure could hurt the API latency. 
> Could have achieved the same by starting a new Thread - but using a message 
> broker is a standard way. 
> 
> 2. Foresight. In case we break down services to separate 
> applications/services, ActiveMQ could be a way to broker the messages. 
> 
> That said, did I understand your question correctly? Or was the question - 
> why specifically ActiveMQ?
> 
> Let me know. 
> Thanks
> 
> On Thu, 19 May 2022 at 21:25, Ádám Sághy <[email protected] 
> <mailto:[email protected]>> wrote:
> Hi Pranjal,
> 
> Its good to meet you! Since you offered your help, I was wondering whether 
> you could help me to get a better insight about what was the reason to use 
> activeMq in the Notification sub-system.
> 
> Regards,
> Adam
> 
>> On 19 May 2022, at 17:48, Arnold Galovics <[email protected] 
>> <mailto:[email protected]>> wrote:
>> 
>> Hi Pranjal,
>> 
>> Got it and I concluded the same from the presentation Ed shared.
>> 
>> Frankly speaking, I'm still not convinced that the "pre-computation of 
>> recipients" is actually needed. I think that asking the question of "who are 
>> the recipients for this notification" every time is reasonable. And since 
>> the notification sending is happening on an asynchronous execution thread, 
>> it won't affect any of the real-time functionalities.
>> 
>> I'd say if I have to put the complexity and the performance benefit onto 
>> balance, I'd say with the current implementation it hangs more towards 
>> complexity.
>> 
>> If there's a performance issue in the future, we can still revisit.
>> 
>> Hope that makes sense.
>> Best,
>> Arnold
>> 
>> On Thu, May 19, 2022 at 5:37 PM Pranjal Goswami <[email protected] 
>> <mailto:[email protected]>> wrote:
>> Hi Arnold, 
>> 
>> Introduction: I designed the Notification sub-system based on what we used 
>> at Superset <https://joinsuperset.com/>.
>> 
>> You are indeed partly right. The notification generation and delivery do not 
>> need to depend on topics and topic_subscribers. 
>> 
>> However, the topics and topic_subscribers are meant to avoid computation of 
>> the list of recipients of the said notifications. Let me explain using a 
>> couple of examples. 
>> 
>> In the context of Superset, a tool to manage recruitments for colleges, if 
>> we want to send a notification to all students of Humanities major, we will 
>> have to get the list of user IDs of the said predicate. 
>> 
>> Now, I can do something like:
>> SELECT ids from users where (.... some joins ....) user.major = 'Humanities' 
>>  and then pass the list of IDs 
>> 
>> OR 
>> 
>> Have a pre-computed list of user IDs for this frequently used group. This 
>> saves me the compute time for getting the query results. The real benefits 
>> come when the real-time fetch of the user IDs using the query logic is 
>> convoluted. 
>> 
>> In the context of Mifos, it could be notifying all Loan Officers of a branch.
>> 
>> The second advantage is the notification being visible to someone who joins 
>> the group later. This is not implemented yet. 
>> Let's say you sent a notification to all the loan officers and a loan 
>> officer joined the next day and should be shown the same message when he 
>> logs in. Then posting a notification to a topic would make sense. This would 
>> make even more sense if we have a message board or a notice board 
>> implemented for important announcements to the members. 
>> 
>> In conclusion, if the feature is being used just with a notification for a 
>> list of User IDs, we don't need topics and subscribers. In case we are 
>> running into cases where we would like to not compute the recipient list 
>> again and again (sort of a cache for recipients of a logical group) then we 
>> can promote the use of topic and topic_subscribers. 
>> 
>> Let me know if there are any questions. 
>> 
>> Thanks
>> 
>> 
>> 
>> 
>> 
>> 
>> 
>> 
>> 
>> On Thu, 19 May 2022 at 12:35, Arnold Galovics <[email protected] 
>> <mailto:[email protected]>> wrote:
>> Hi Ed,
>> 
>> Wow, that's great you were able to grab the docs and the email thread.
>> I checked both and it confirms my theory. The topic and topic_subscriber 
>> tables were unnecessary in the picture to achieve the behavior we wanted.
>> 
>> @Victor: thanks for the info.
>> 
>> Best,
>> Arnold
>> 
>> On Wed, May 18, 2022 at 5:22 PM VICTOR MANUEL ROMERO RODRIGUEZ 
>> <[email protected] <mailto:[email protected]>> wrote:
>> Hello Arnold,
>> 
>> For the classical UI some events were not passed to the UI (details URL) 
>> because the Event property names were not matching. 
>> 
>> I think it is still open .. 
>> 
>> https://github.com/openMF/community-app/pull/3441 
>> <https://github.com/openMF/community-app/pull/3441>
>> 
>> Regards
>> 
>> Victor
>> 
>> El mié, 18 may 2022 a las 10:10, Arnold Galovics (<[email protected] 
>> <mailto:[email protected]>>) escribió:
>> Hi guys,
>> 
>> Thanks for the feedback. I can only agree with you, we don't want to lose 
>> features that are potentially used.
>> 
>> I was probably not crystal clear when I mentioned the term "notification 
>> topic". I was mainly referring to the "topic" and "topic_subscriber" tables 
>> which are currently not exposed through APIs but only used for our internal 
>> purpose. However, after spending some time on understanding what the purpose 
>> was behind this, I figured out that these tables and their related 
>> functionality is not even needed to maintain the completeness of our feature 
>> set so I was able to replace it fairly easily and get rid of the related 
>> code.
>> 
>> I'm still polishing the PR, but you can look at it here: 
>> https://github.com/apache/fineract/pull/2330 
>> <https://github.com/apache/fineract/pull/2330>
>> 
>> Just as a side note, these 2 tables are also used in conjunction with the UI 
>> notifications (on the top of the UI) and I realized that the UI notification 
>> feature is not even working without a running ActiveMQ broken because the 
>> logic is buggy.
>> 
>> I managed to fix that as well as part of the cleanup and I introduced a new 
>> test-case to verify this logic.
>> 
>> Summary:
>> - ~1500 less lines of unnecessary code
>> - A simplified notification implementation
>> - New test case to verify UI notifications
>> 
>> Best,
>> Arnold
>> 
>> 
>> On Wed, May 18, 2022 at 5:02 PM James Dailey <[email protected] 
>> <mailto:[email protected]>> wrote:
>> Yes, we have to be careful about removing things, that is probably an 
>> unspoken principle on this project as we don't know how it's being used. 
>> (unfortunately)  
>> However, if it makes sense from an architectural perspective to rationalize 
>> the notification and event handling frameworks, then I would suggest that we 
>> find a way to migrate this behavior.  
>> ... and wondering if this belongs in its own extension or outside component. 
>> 
>> It may be "half-implemented" but that doesn't mean it isn't being used. ;) 
>> 
>> 
>> On Wed, May 18, 2022 at 7:43 AM Bharath Gowda <[email protected] 
>> <mailto:[email protected]>> wrote:
>> Hi Arnold,
>> 
>> I have some organizations using the notification feature effectively for 
>> sanctioning or disbursing the loan accounts based on the notifications which 
>> they receive. It is mainly useful when there are different levels of 
>> approvals for the loan cycle.
>> 
>> (user configuration document for this feature is here 
>> <https://docs.mifos.org/mifosx/user-manual/for-administrators-mifos-x-platform/configure-notifications>)
>> 
>> Regards,
>> Bharath
>> Lead Implementation Analyst | Mifos Initiative
>> Skype: live:cbharath4| Mobile: +91.7019635592
>> http://mifos.org <http://mifos.org/>  <http://facebook.com/mifos>  
>> <http://www.twitter.com/mifos>
>> 
>> 
>> On Wed, May 18, 2022 at 3:29 PM Aleksandar Vidakovic 
>> <[email protected] <mailto:[email protected]>> wrote:
>> ... thanks Adam... learned again.
>> 
>> On Wed, May 18, 2022 at 10:28 AM Ádám Sághy <[email protected] 
>> <mailto:[email protected]>> wrote:
>> Hi guys,
>> 
>> So lets see what i know about this functionality:
>> 
>> - The Fineract is the publisher and also a listener for the Notification 
>> events.
>> 
>> - Fineract is publishing notifications in the following situations:
>> "ACTIVATE_CLIENT"
>> "ACTIVATE_CENTER"
>> "ACTIVATE_GROUP"
>> "READ_SAVINGSACCOUNT"
>> "READ_DIVIDEND_SHAREPRODUCT"
>> "APPROVE_FIXEDDEPOSITACCOUNT"
>> "APPROVE_RECURRINGDEPOSITACCOUNT"
>> "ACTIVATE_FIXEDDEPOSITACCOUNT"
>> "ACTIVATE_RECURRINGDEPOSITACCOUNT
>> "ACTIVATE_SAVINGSACCOUNT"
>> "READ_SAVINGSACCOUNT"
>> "APPROVE_LOAN", 
>> "DISBURSE_LOAN"
>> "READ_LOAN"
>> "READ_Rescheduled Loans"
>> "READ_LOAN"
>> "READ_LOANPRODUCT"
>> "APPROVE_SAVINGSACCOUNT"
>> "READ_SAVINGSACCOUNT"
>> "APPROVE_SHAREACCOUNT"
>> “ACTIVATE_SHAREACCOUNT”
>> 
>> - There is a topic subsciption functionality also where appusers can 
>> subscribe for events and they got notified when that event occured.
>> - When a user is created based on the roles, the fineract might subscribe 
>> automatically for topics
>> 
>> - Fineract listener are listening for these kind of events and when it 
>> happens it will create a notification entry into the database for the 
>> subscribed users.
>> - When a subscribed user logs into the Fineract, they will get the 
>> notification (on the UI for example a popup).
>> 
>> I hope it helps to visualize this functionality a little bit better and  
>> decide on its future.
>> 
>> Regards,
>> Adam
>> 
>>> On 18 May 2022, at 09:56, Aleksandar Vidakovic <[email protected] 
>>> <mailto:[email protected]>> wrote:
>>> 
>>> ... other question: does it do anything? I'll have another look at it 
>>> today, but it seems non-functional.
>>> 
>>> It's going to be hard to reach in general a consensus if people are not 
>>> participating... same argument could be made for introducing Liquibase; I'm 
>>> sure that others invested time in Flyway, but we still replaced it.
>>> 
>>> Just my 2 cents.
>>> 
>>> On Wed, May 18, 2022 at 9:43 AM Awasum Yannick <[email protected] 
>>> <mailto:[email protected]>> wrote:
>>> Hello Aleks and Arnold,
>>> 
>>> I won't remove that feature given we don't know who may or may not be using 
>>> it.
>>> 
>>> There are people using Fineract who are not even on this dev list or 
>>> participating in conversations.
>>> 
>>> I would be careful with what I remove even if it looks unusable to me.
>>> 
>>> On Wed, May 18, 2022, 02:11 Aleksandar Vidakovic <[email protected] 
>>> <mailto:[email protected]>> wrote:
>>> I would say: +1
>>> 
>>> On Tue, May 17, 2022 at 10:01 PM Arnold Galovics <[email protected] 
>>> <mailto:[email protected]>> wrote:
>>> Hi guys,
>>> 
>>> I'm exploring the current event handling frameworks available in Fineract - 
>>> Hooks, Business events and Notification events - and I was wondering if 
>>> anybody is using the so called "topic subscriptions" in Fineract within the 
>>> Notification events module.
>>> 
>>> As far as I can tell, it's a half-complete implementation but I see that 
>>> upon creating a new user and assigning it to an office, it automatically 
>>> subscribes the user to a particular topic but the notion of "subscribing to 
>>> a topic" doesn't really have any meaning at this point.
>>> 
>>> If nobody is using the feature, I'll just remove it to get rid of some of 
>>> the weight we've been carrying.
>>> 
>>> Let me know.
>>> 
>>> Best,
>>> Arnold
>> 
>> 
>> 
>> -- 
>> Regards,
>> Pranjal Goswami
>> 
> 
> 
> 
> -- 
> Regards,
> Pranjal Goswami
> 

Reply via email to