Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NXDRIVE-2889: Display system notification for document review(Bug Fix) #5001

78 changes: 66 additions & 12 deletions nxdrive/client/workflow/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from datetime import datetime, timedelta, timezone
from datetime import datetime, timezone
from logging import getLogger
from typing import Dict, List

Expand All @@ -14,6 +14,8 @@
class Workflow:
"""Workflow Management for document Review"""

user_task_list = {}

def __init__(self, remote: "Remote") -> None:
self.remote = remote

Expand All @@ -25,43 +27,95 @@

response = self.remote.documents.get(doc_id)
if response:
engine.send_task_notification(task_id, response.path)
# check if user have to choose participants
if "chooseParticipants" in first_task.directive:
engine.send_task_notification(
task_id, response.path, "Choose Participants"
)
else:
engine.send_task_notification(task_id, response.path, "Review Document")
else:
log.error("Failed to fetch document details.")

def update_user_task_data(self, tasks: List[Task], userId: str) -> List[Task]:
"""Update user_task_list for below scenarios
swetayadav1 marked this conversation as resolved.
Show resolved Hide resolved
1. Add new key if it doesn't exist in user_task_list
2. If user_task_list[a_user] have [tasks_a, tasks_b] and got tasks[tasks_a, tasks_c] then
a. Add tasks_c in user_task_list[a_user] and send notification.
b. Remove tasks_b from user_task_list[a_user] and no notification in this case
3. If user_task_list[a_user] have [tasks_a, tasks_b] and got tasks[tasks_a, tasks_b].
In this case no need to the send notification
"""
task_ids = [task.id for task in tasks]

if userId not in self.user_task_list:
self.user_task_list[userId] = task_ids
return tasks
# Get the existing task IDs for the user
existing_task_ids = set(self.user_task_list[userId])

# Determine new tasks added for the user
new_task_ids = set(task_ids).difference(existing_task_ids)
if new_task_ids:
self.user_task_list[userId] = task_ids
return [task for task in tasks if task.id in new_task_ids]

# Determine old/completed tasks to be removed
old_task_ids = existing_task_ids.difference(task_ids)
if old_task_ids:
self.user_task_list[userId] = [

Check warning on line 66 in nxdrive/client/workflow/__init__.py

View check run for this annotation

Codecov / codecov/patch

nxdrive/client/workflow/__init__.py#L66

Added line #L66 was not covered by tests
id for id in existing_task_ids if id not in old_task_ids
]
return []

Check warning on line 69 in nxdrive/client/workflow/__init__.py

View check run for this annotation

Codecov / codecov/patch

nxdrive/client/workflow/__init__.py#L69

Added line #L69 was not covered by tests

# If no new tasks added or removed
return []

def get_pending_tasks(self, engine: Engine, initial_run: bool = True) -> List[Task]: # type: ignore
"""Get Tasks for document review"""
try:
options = {"userId": self.remote.user_id}
options = {"userId": engine.remote.user_id}
tasks = self.remote.tasks.get(options)

if tasks:
if not initial_run:
tasks = self.filter_tasks(tasks)
tasks = self.remove_overdue_tasks(tasks)
if tasks:
tasks = self.update_user_task_data(tasks, options["userId"])

if tasks:
if len(tasks) > 1:
# Send generic notification for multiple tasks
engine.send_task_notification(
tasks[0].targetDocumentIds[0]["id"], ""
tasks[0].targetDocumentIds[0]["id"], "", "Review Document"
)
else:
# Fetch document data
self.fetch_document(tasks, engine)
else:
log.info("No Task for processing...")
else:
self.clean_user_task_data(options["userId"])
log.info("No Task for processing...")
except Exception as exec:
log.error(f"Exception occurred while Fetching Tasks: {exec}")

@staticmethod
def filter_tasks(tasks: List[Task]) -> List[Task]:
"""Filter new tasks created within the last hour"""
last_hour = datetime.now(tz=timezone.utc) - timedelta(minutes=60)
log.info("Filtering tasks created in the last hour.")
def remove_overdue_tasks(tasks: List[Task]) -> List[Task]:
"""Remove overdue tasks"""
current_time = datetime.now(tz=timezone.utc)
log.info("Remove overdue tasks")
return [
task
for task in tasks
if datetime.strptime(task.created, "%Y-%m-%dT%H:%M:%S.%f%z") > last_hour
if (
datetime.strptime(task.dueDate, "%Y-%m-%dT%H:%M:%S.%f%z") > current_time
)
]

def clean_user_task_data(self, userId: str = "", /) -> None:
"""Remove user data for below scenarios:
1. If no tasks assigned to user remove the ID
2. If account has been removed than remove the ID if it exist
"""
if userId and userId in self.user_task_list.keys():
self.user_task_list.pop(userId)
return
1 change: 1 addition & 0 deletions nxdrive/data/i18n/i18n.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
"CONTEXT_MENU_4": "Upload content",
"CONTINUE": "Continue",
"CONTINUE_USING": "Continue using Drive %1",
"CHOOSE_PARTICIPANTS": "Choose Participants %1",
"CREATE": "Create",
"CREATE_REPORT": "Generate bug report",
"DATETIME_FORMAT": "%x %X",
Expand Down
8 changes: 5 additions & 3 deletions nxdrive/engine/engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ class Engine(QObject):
directTransferNewFolderError = pyqtSignal()
directTransferNewFolderSuccess = pyqtSignal(str)
directTransferSessionFinished = pyqtSignal(str, str, str)
displayPendingTask = pyqtSignal(str, str, str)
displayPendingTask = pyqtSignal(str, str, str, str)

type = "NXDRIVE"
# Folder locker - LocalFolder processor can prevent
Expand Down Expand Up @@ -1600,8 +1600,10 @@ def get_user_full_name(self, userid: str, /, *, cache_only: bool = False) -> str

return full_name

def send_task_notification(self, task_id: str, remote_path: str, /) -> None:
self.displayPendingTask.emit(self.uid, task_id, remote_path)
def send_task_notification(
self, task_id: str, remote_path: str, notification_title: str, /
) -> None:
self.displayPendingTask.emit(self.uid, task_id, remote_path, notification_title)


@dataclass
Expand Down
9 changes: 7 additions & 2 deletions nxdrive/gui/application.py
Original file line number Diff line number Diff line change
Expand Up @@ -398,8 +398,13 @@
if feature.restart_needed:
self.manager.restartNeeded.emit()

if feature.enabled and feature == self.tasks_management_feature_model:
self.init_workflow()
if feature == self.tasks_management_feature_model:
if feature.enabled:

Check warning on line 402 in nxdrive/gui/application.py

View check run for this annotation

Codecov / codecov/patch

nxdrive/gui/application.py#L401-L402

Added lines #L401 - L402 were not covered by tests
# Check for tasks while enabling the feature
self.init_workflow()

Check warning on line 404 in nxdrive/gui/application.py

View check run for this annotation

Codecov / codecov/patch

nxdrive/gui/application.py#L404

Added line #L404 was not covered by tests
else:
# clean user_task_list if we are disabling the tasks_management feature
Workflow.user_task_list = {}

Check warning on line 407 in nxdrive/gui/application.py

View check run for this annotation

Codecov / codecov/patch

nxdrive/gui/application.py#L407

Added line #L407 was not covered by tests

def _center_on_screen(self, window: QQuickView, /) -> None:
"""Display and center the window on the screen."""
Expand Down
6 changes: 6 additions & 0 deletions nxdrive/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
from .autolocker import ProcessAutoLockerWorker
from .client.local import LocalClient
from .client.proxy import get_proxy, load_proxy, save_proxy, validate_proxy
from .client.workflow import Workflow
from .constants import (
APP_NAME,
DEFAULT_CHANNEL,
Expand Down Expand Up @@ -942,6 +943,11 @@
if not engine:
return

if Feature.tasks_management:

Check warning on line 946 in nxdrive/manager.py

View check run for this annotation

Codecov / codecov/patch

nxdrive/manager.py#L946

Added line #L946 was not covered by tests
# Clean user tasks data incase we are unbinding the account.
self.workflow = Workflow(engine.remote)
self.workflow.clean_user_task_data(engine.remote.user_id)

Check warning on line 949 in nxdrive/manager.py

View check run for this annotation

Codecov / codecov/patch

nxdrive/manager.py#L948-L949

Added lines #L948 - L949 were not covered by tests

engine.unbind()
self.dao.delete_engine(uid)

Expand Down
31 changes: 25 additions & 6 deletions nxdrive/notification.py
Original file line number Diff line number Diff line change
Expand Up @@ -522,12 +522,24 @@ def __init__(self, engine_uid: str, /) -> None:
class DisplayPendingTask(Notification):
"""Display a notification for pending tasks"""

def __init__(self, engine_uid: str, remote_ref: str, remote_path: str, /) -> None:
def __init__(
self,
engine_uid: str,
remote_ref: str,
remote_path: str,
notification_title: str,
/,
) -> None:
values = [remote_path]
description = (
"PENDING_DOCUMENT_REVIEWS"
if notification_title == "Review Document"
else "CHOOSE_PARTICIPANTS"
)
super().__init__(
uid="PENDING_DOCUMENT_REVIEWS",
title="Review Document",
description=Translator.get("PENDING_DOCUMENT_REVIEWS", values=values),
uid=description,
title=notification_title,
description=Translator.get(description, values=values),
level=Notification.LEVEL_INFO,
flags=(
Notification.FLAG_PERSISTENT
Expand Down Expand Up @@ -663,6 +675,13 @@ def _invalidAuthentication(self) -> None:
self.send_notification(notif)

def _display_pending_task(
self, engine_uid: str, remote_ref: str, remote_path: str, /
self,
engine_uid: str,
remote_ref: str,
remote_path: str,
notification_title: str,
/,
) -> None:
self.send_notification(DisplayPendingTask(engine_uid, remote_ref, remote_path))
self.send_notification(
DisplayPendingTask(engine_uid, remote_ref, remote_path, notification_title)
)
13 changes: 12 additions & 1 deletion tests/functional/test_engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,18 @@ def test_send_task_notification(manager_factory):
manager, engine = manager_factory()
Translator(find_resource("i18n"), lang="en")
with manager:
assert engine.send_task_notification(str(uuid4()), "/doc_path/test.txt") is None
assert (
engine.send_task_notification(
str(uuid4()), "/doc_path/test.txt", "Review Document"
)
is None
)
assert (
engine.send_task_notification(
str(uuid4()), "/doc_path/test.txt", "Choose Participants"
)
is None
)


def test_get_task_url(manager_factory, nuxeo_url):
Expand Down
71 changes: 55 additions & 16 deletions tests/unit/test_workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,11 @@
@pytest.fixture()
def task():
task = Task
task.id = str(uuid4())
task.id = "taskid_test"
task.targetDocumentIds = [{"id": f"{uuid4()}"}]
datetime_30_min_ago = datetime.now(tz=timezone.utc) - timedelta(minutes=30)
task.created = datetime_30_min_ago.strftime("%Y-%m-%dT%H:%M:%S.%f%z")
datetime_30_min_ago = datetime.now(tz=timezone.utc) + timedelta(minutes=30)
task.dueDate = datetime_30_min_ago.strftime("%Y-%m-%dT%H:%M:%S.%f%z")
task.directive = ""
return task


Expand All @@ -38,6 +39,12 @@ def workflow(remote):
return Workflow(remote)


@pytest.fixture()
def engine(engine, remote):
engine.remote = remote
return engine


@pytest.fixture()
def application(manager, workflow):
application = Mock(spec=Application)
Expand Down Expand Up @@ -66,45 +73,77 @@ def workflow_worker(manager, application, workflow):


def test_get_pending_task_for_no_doc(workflow, engine, remote):
# No response from api for pending task
remote.tasks.get = Mock(return_value=[])
# No response from api for pending task for user_a
assert Workflow.user_task_list == {}

remote.user_id = "user_a"
engine.remote = remote
engine.remote.tasks.get = Mock(return_value=[])
assert workflow.get_pending_tasks(engine) is None

# If no tasks assigned to user remove the ID
Workflow.user_task_list = {"user_a": ["taskid_1"]}
engine.remote.tasks.get = Mock(return_value=[])
assert workflow.get_pending_tasks(engine) is None
assert Workflow.user_task_list == {}
swetayadav1 marked this conversation as resolved.
Show resolved Hide resolved


def test_get_pending_task_for_single_doc(workflow, engine, task, remote):
# Triggered through polling for single task
remote.tasks.get = Mock(return_value=[task])
# Triggered through polling for single task,
# Add new key if it doesn't exist in user_task_list
remote.user_id = "user_a"
engine.remote = remote
engine.remote.tasks.get = Mock(return_value=[task])
workflow.fetch_document = Mock()
assert workflow.get_pending_tasks(engine, False) is None
assert Workflow.user_task_list == {"user_a": ["taskid_test"]}


def test_get_pending_task_for_multiple_doc(workflow, engine, task, remote):
# Triggered through polling for multiple task
remote.tasks.get = Mock(return_value=[task, task])
task1 = task
task1.id = "taskid_1"
task2 = task
task2.id = "taskid_2"
remote.user_id = "user_a"
engine.remote = remote
engine.remote.tasks.get = Mock(return_value=[task1, task2])
engine.send_task_notification = Mock()
assert workflow.get_pending_tasks(engine, False) is None
assert Workflow.user_task_list == {"user_a": ["taskid_2", "taskid_2"]}

# user_task_list[a_user] have [tasks_a, tasks_b] and got tasks[tasks_a, tasks_b].
# In this case no need to the send notification
assert workflow.get_pending_tasks(engine, False) is None
assert Workflow.user_task_list == {"user_a": ["taskid_2", "taskid_2"]}


def test_filtered_task(workflow, engine, task, remote):
def test_remove_overdue_tasks(workflow, engine, task, remote):
# No new task during an hour
datetime_30_min_ago = datetime.now(tz=timezone.utc) - timedelta(minutes=160)
task.created = datetime_30_min_ago.strftime("%Y-%m-%dT%H:%M:%S.%f%z")
remote.tasks.get = Mock(return_value=[task])
datetime_1_hr_ago = datetime.now(tz=timezone.utc) - timedelta(minutes=160)
task.dueDate = datetime_1_hr_ago.strftime("%Y-%m-%dT%H:%M:%S.%f%z")
engine.remote.tasks.get = Mock(return_value=[task])
engine.send_task_notification = Mock()
assert workflow.get_pending_tasks(engine, False) is None

# raise exception
task.created = None
task.dueDate = None
assert workflow.get_pending_tasks(engine, False) is None


def test_fetch_document(workflow, engine, task, remote):
remote.documents.get = Mock(path="/doc_path/doc.txt")
def test_fetch_document(workflow, engine, task):
engine.remote.documents.get = Mock(path="/doc_path/doc.txt")
swetayadav1 marked this conversation as resolved.
Show resolved Hide resolved
engine.send_task_notification = Mock()
workflow.fetch_document([task], engine)

# No response from remote.documents.get
remote.documents.get = Mock(return_value=None)
engine.remote.documents.get = Mock(return_value=None)
workflow.fetch_document([task], engine)

# send notification for directive
task.directive = "chooseParticipants"
engine.remote.documents.get = Mock(path="/doc_path/doc.txt")
engine.send_task_notification = Mock()
workflow.fetch_document([task], engine)


Expand Down
Loading