SAK-49949 tasks Soft delete tasks for soft deleted sites #12945
Conversation
| @@ -21,11 +21,11 @@ | |||
| <bean id="org.sakaiproject.tasks.api.TaskService" | |||
| class="org.sakaiproject.tasks.impl.TaskServiceImpl" init-method="init"> | |||
|
|
|||
| <property name="transactionTemplate"> | |||
| <!--property name="transactionTemplate"> | |||
|
@ern SOFT_DELETED because they mimic site soft deletion. If the sites are restored, then the tasks are too. |
|
Yeah I understand that but I am not convinced it makes sense to mark the data as softly deleted if the site is softly deleted. Simply checking if the site is softly deleted (SAKAI_SITE.IS_SOFTLY_DELETED) should be enough to say don't load tasks from this site. |
|
Ok, fair enough. I can do that. |
|
@ern Updating to check all the sites when loading up the synoptic tasks would take potentially a lot of calls to the site service. I know they are cached, but still. So, a user may have a set of tasks from 5 sites. Everytime getCurrentTasksForCurrent user is called, that would mean a site retrieval for each task. Caching would help, of course, but it still feels heavy. So, it's a tradeoff between keeping soft deleted in sync between tasks and the site, with a DDL change, or a potentially heavy hit on the site service. Tasks will be called everytime a user hits the dashboard. |
| @@ -95,7 +98,10 @@ public void update(Observable o, Object arg) { | |||
| if (event.getEvent().equals(SiteService.SECURE_UPDATE_SITE_MEMBERSHIP)) { | |||
| try { | |||
| Set<String> siteUsers = siteService.getSite(event.getContext()).getUsers(); | |||
There was a problem hiding this comment.
I know this is out of scope for your PR, just an FYI for anyone following. event.getContext() is usually null here. also it can get called thousands of times in quick succession during SIS updates ;)
There was a problem hiding this comment.
So basically, if students are being added one by one, this will be a big hit as it will be pulling the list of students from the site on every event?
There was a problem hiding this comment.
i think it's just an annoyance as this can be a big NPE .... i'm okay with the getUsers pull because it should be cached
There was a problem hiding this comment.
Does it get cached though, if the site members have been updated before the event was fired? Anyway, how often would a big SIS update like that happen?
https://sakaiproject.atlassian.net/browse/SAK-49949