somaliaims / Somali-AIMSUI

GNU Affero General Public License v3.0
1 stars 3 forks source link

Project deletion requests - and manager users - when do they get a) notification b) email #600

Open matmaxgeds opened 4 years ago

matmaxgeds commented 4 years ago

The stranger part is that I got an email about it. An email should come at the point where there is a notification, not before. This suggests that the email system is running on a different logic to the notifications?

The conditions (business rules) in the code says this:

Edge case:- If a project has only funder or implementer and not other organization is involved or if there are organizations involved but only of them is registered who actually initiated the request. Then the notification might appear to manager users after 7 days.

No, this is not the edge case. I thought over it and below I have given the reasons. The easy at this stage was to forward Project deletion requests to manager users straightaway. The edge case needs lot of query calculations while showing the notifications count. For example:

  1. For every project we need to get the list of funders.
  2. List of Implementers
  3. Then for each funder we need to check either it has users.
  4. Same for the implementers.
  5. If not, take that as count for the manager and show the notification else not.
  6. Similarly, same to check while sending email for project deletion request.

    We can discuss it in the upcoming meeting.

Solution

  1. on request, send to all funders and implementers (just use the API to get a list of all their users (even if there are none) - no logic needed), except the requester org, if it is still live after 7 days, send it to the management users, don't worry about the case where there are no other orgs,
  2. email and notification should be sent at the same time i.e. from the same trigger
  3. The time delay should be configurable in a management page e.g. how many days - see another issue on this
raashidahmad commented 3 years ago

After studying the Implementation of this feature, I am listing the steps below for confirmation:

  1. On a project deletion request, a notification email will be sent to all the related funders and Implementers except the one who requested the deletion.
  2. If the request does not get any feedback in 7 days. Then an email to the management users will be sent, and notification will be visible to management users.
  3. We need a mechanism where email is sent to the management user just once, as this check will be executed every mid night for any pending requests. For this purpose I will add a new column to the project deletion requests.
  4. I will design a page where this duration of 7 days could be configured.
matmaxgeds commented 3 years ago

@raashidahmad this sounds good - it would be sensible if the 7 day period applied to all the cases we have where a management user is notified if another user has not taken action e.g. joining a project, deleting a project, approving a user etc - I wonder why we have not come onto this issue before - so maybe it would be best to add the column to the notifications table e.g. 'initial trigger date' rather than needing it on all the different tables?

raashidahmad commented 3 years ago

@matmaxgeds Thanks for your sharing you ideas. Agree with your thoughts on managing days for notifications through a management page. However, we cannot keep the date for all the cases in notifications table. The reason for these are:

  1. We are keeping different things in their own table. For example, user registration requests, project deletion requests, project merge requests.
  2. Upon trigger, we need specific data from different tables
  3. From database design point of view we cannot reference keys for a single column from different tables.
  4. We have dates in different tables, and we will keep management section to get number of days passed to trigger the events.
matmaxgeds commented 3 years ago

@raashidahmad understood - if we are also not allowing configuration of the delay anywhere else, we should also not do it here