Merge pull request #66 from briehl/fetch-resource-info

Fix problem with groups notifications being sent to "target" users
kbase · Mar 21, 2019 · 315ad52 · 315ad52
2 parents 8dffc2f + 9e49756
commit 315ad52
Show file tree

Hide file tree

Showing 10 changed files with 77 additions and 14 deletions.
diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
@@ -1,8 +1,11 @@
 KBase Feeds Service
 
+### Version 1.0.1
+- Fix issue where groups notifications were being seen by users in the target field as well as the users list.
+
 ### Version 1.0.0
- - First public release!
- - Supports adding notifications from other KBase services.
- - Notifications are sent directly to users, no subscription model yet.
- - Notifications can also be binned into groups.
- - See README.md for API details.
+- First public release!
+- Supports adding notifications from other KBase services.
+- Notifications are sent directly to users, no subscription model yet.
+- Notifications can also be binned into groups.
+- See README.md for API details.
diff --git a/deployment/conf/.templates/deploy.cfg.templ b/deployment/conf/.templates/deploy.cfg.templ
@@ -38,3 +38,12 @@ workspace-url = {{ default .Env.workspace_url "https://ci.kbase.us/services/ws"
 groups-url = {{ default .Env.groups_url "https://ci.kbase.us/services/groups" }}
 njs-url = {{ default .Env.njs_url "https://ci.kbase.us/services/njs_wrapper" }}
 nms-url = {{ default .Env.nms_url "https://ci.kbase.us/services/narrative_method_store/rpc" }}
+
+# Service names for notification sources.
+# These are the expected mappings for source services to do special management (fanning out notifications, etc).
+# Any source not used here won't be recognized and will do some default things.
+service-groups = {{ default .Env.service_groups "groupsservice" }}
+service-workspace = {{ default .Env.service_ws "workspaceservice" }}
+service-narrative = {{ default .Env.service_narrative "narrativeservice" }}
+service-jobs = {{ default .Env.service_jobs "jobservice" }}
+service-kbase = {{ default .Env.servivce_kbase "kbase" }}
diff --git a/deployment/deploy.cfg.example b/deployment/deploy.cfg.example
@@ -39,3 +39,12 @@ default-note-count = 100
 # Useful for testing, etc.
 # SET TO FALSE IN PRODUCTION!
 debug=False
+
+# Service names for notification sources.
+# These are the expected mappings for source services to do special management (fanning out notifications, etc).
+# Any source not used here won't be recognized and will do some default things.
+service-groups = groupsservice
+service-workspace = workspaceservice
+service-narrative = narrativeservice
+service-jobs = jobservice
+service-kbase = kbase
diff --git a/deployment/docker-compose.yml b/deployment/docker-compose.yml
@@ -13,6 +13,11 @@ services:
       - nms-url=https://ci.kbase.us/services/narrative_method_store/rpc
       - AUTH_TOKEN=fake_token
       - default-note-count=100
+      - service-groups = groupsservice
+      - service-workspace = workspaceservice
+      - service-narrative = narrativeservice
+      - service-jobs = jobservice
+      - service-kbase = kbase
     command:
       - "-wait"
       - "tcp://ci-mongo:27017"

diff --git a/feeds/config.py b/feeds/config.py
@@ -24,6 +24,11 @@
 KEY_WS_URL = "workspace-url"
 KEY_NMS_URL = "nms-url"
 KEY_DEFAULT_COUNT = "default-note-count"
+KEY_SERVICE_GROUPS = "service-groups"
+KEY_SERVICE_WORKSPACE = "service-workspace"
+KEY_SERVICE_NARRATIVE = "service-narrative"
+KEY_SERVICE_JOBS = "service-jobs"
+KEY_SERVICE_KBASE = "service-kbase"
 
 
 class FeedsConfig(object):
@@ -68,11 +73,21 @@ def __init__(self):
             self.debug = False
         else:
             self.debug = True
+
+        # URLs
         self.auth_url = self._get_line(cfg, KEY_AUTH_URL)
         self.njs_url = self._get_line(cfg, KEY_NJS_URL)
         self.ws_url = self._get_line(cfg, KEY_WS_URL)
         self.groups_url = self._get_line(cfg, KEY_GROUPS_URL)
         self.nms_url = self._get_line(cfg, KEY_NMS_URL)
+
+        # Source service names
+        self.service_groups = self._get_line(cfg, KEY_SERVICE_GROUPS)
+        self.service_workspace = self._get_line(cfg, KEY_SERVICE_WORKSPACE)
+        self.service_narrative = self._get_line(cfg, KEY_SERVICE_NARRATIVE)
+        self.service_jobs = self._get_line(cfg, KEY_SERVICE_JOBS)
+        self.service_kbase = self._get_line(cfg, KEY_SERVICE_KBASE)
+
         self.default_max_notes = self._get_line(cfg, KEY_DEFAULT_COUNT)
         try:
             self.default_max_notes = self._get_line(cfg, KEY_DEFAULT_COUNT)

diff --git a/feeds/managers/notification_manager.py b/feeds/managers/notification_manager.py
@@ -18,6 +18,7 @@
 from .fanout_modules.jobs import JobsFanout
 from .fanout_modules.kbase import KBaseFanout
 from feeds.entity.entity import Entity
+from feeds.config import get_config
 
 
 class NotificationManager(BaseManager):
@@ -46,19 +47,20 @@ def get_target_users(self, note: Notification) -> List[Entity]:
         TODO: add adapters, maybe subclass notifications to handle each source?
         """
         fanout = None
-        if note.source == 'ws' or note.source == 'workspace':
+        cfg = get_config()
+        if note.source == cfg.service_workspace:
             fanout = WorkspaceFanout(note)
-        elif note.source == 'groups':
+        elif note.source == cfg.service_groups:
             fanout = GroupsFanout(note)
-        elif note.source == 'jobs':
+        elif note.source == cfg.service_jobs:
             fanout = JobsFanout(note)
-        elif note.source == 'kbase':
+        elif note.source == cfg.service_kbase:
             fanout = KBaseFanout(note)
 
         if fanout is not None:
             user_list = fanout.get_target_users()
         else:
-            user_list = list(set(note.users + note.target))
+            user_list = list(note.users)
 
         return user_list
 

diff --git a/feeds/server.py b/feeds/server.py
@@ -33,7 +33,7 @@
     log_error
 )
 
-VERSION = "1.0.0"
+VERSION = "1.0.1"
 
 try:
     from feeds import gitcommit

diff --git a/test/api/test_api_v1.py b/test/api/test_api_v1.py
@@ -128,6 +128,7 @@ def test_get_notifications_groups(client, mock_valid_user_token, mock_valid_user
 ###
 
 def test_post_notification_ok(client, mock_valid_service_token, mock_valid_user_token, mock_valid_users, mock_workspace_info, mongo_notes):
+    # Set up note info.
     service = "a_service"
     test_user = "test_note"
     test_actor = "test_actor"
@@ -136,25 +137,34 @@ def test_post_notification_ok(client, mock_valid_service_token, mock_valid_user_
     mock_valid_service_token("user", "pw", service)
     note = {
         "actor": {"id": test_actor, "type": "user"},
-        "target": [{"id": test_user, "type": "user"}],
+        "users": [{"id": test_user, "type": "user"}],
+        "target": [{"id": test_actor, "type": "user"}],
         "verb": 1,
         "level": 1,
         "object": {"id": "stuff", "type": "workspace"},
         "source": service
     }
+    # Post the test note.
     response = client.post(
         "/api/V1/notification",
         headers={"Authorization": "token-"+str(uuid4())},
         json=note
     )
     post_return = json.loads(response.data)
+    # Make sure we get an id back, keep it so we can validate the user's feed.
     assert 'id' in post_return
     note_id = post_return['id']
     mock_valid_user_token(test_user, "Some Name")
+    # Get the test user's feed
     response = client.get("/api/V1/notifications", headers={"Authorization": "token-"+str(uuid4())})
     data_return = json.loads(response.data)
     assert len(data_return['user']['feed']) == 1
     assert data_return['user']['feed'][0]['id'] == note_id
+    # Get the actor's feed (also the target) - make sure the note isn't there (that it's empty, really).
+    mock_valid_user_token(test_actor, "Some Actor")
+    response = client.get("/api/V1/notifications", headers={"Authorization": "token-"+str(uuid4())})
+    data_return = json.loads(response.data)
+    assert len(data_return['user']['feed']) == 0
 
 
 def test_post_notification_no_auth(client):

diff --git a/test/test.cfg b/test/test.cfg
@@ -14,6 +14,11 @@ global-feed=_kbase_
 debug=False
 lifespan=30
 default-note-count=100
+service-groups=groupsservice
+service-workspace=workspaceservice
+service-narrative=narrativeservice
+service-jobs=jobsservice
+service-kbase=kbase
 
 [test]
 mongo-exe=/usr/local/bin/mongod

diff --git a/test/test_config.py b/test/test_config.py
@@ -22,7 +22,12 @@
     'nms-url=nms',
     'global-feed=global',
     'lifespan=30',
-    'default-note-count=100'
+    'default-note-count=100',
+    'service-groups=groupsservice',
+    'service-workspace=workspaceservice',
+    'service-narrative=narrativeservice',
+    'service-jobs=jobsservice',
+    'service-kbase=kbase'
 ]
 
 @pytest.fixture(scope="function")
@@ -124,7 +129,7 @@ def test_config_from_env_ok(dummy_config, dummy_auth_token):
     assert cfg.debug == False
     del os.environ['KB_DEPLOYMENT_CONFIG']
     if kb_dep_config is not None:
-        os.environ['KB_DEPLOYMENT_CONFIG'] = path_backup
+        os.environ['KB_DEPLOYMENT_CONFIG'] = kb_dep_config
     if feeds_config_backup is not None:
         os.environ['FEEDS_CONFIG'] = feeds_config_backup