Merge pull request #45 from jrsmth/44_scroll_duplicate_fix

[#44] Prevent duplicate records for the same streak

documentation/CHANGELOG.md

@@ -12,8 +12,9 @@
 # Releases
 <!-- @LatestFirst -->
 
-## [0.3.1] [![user](https://img.shields.io/badge/jrsmth-181717.svg?style=flat&logo=github)](https://github.com/jrsmth)
-[Scroll Command](https://github.com/jrsmth/waffle-bot/milestone/4) (08/04/2024)
+## [0.3.1] [![user](https://img.shields.io/badge/adamj335-181717.svg?style=flat&logo=github)](https://github.com/adamj335) [![user](https://img.shields.io/badge/jrsmth-181717.svg?style=flat&logo=github)](https://github.com/jrsmth)
+[Scroll Command](https://github.com/jrsmth/waffle-bot/milestone/4) (11/04/2024)
+- `#44` Prevent duplicate records for the same streak
 - `#46` Add README Badges (Code Coverage, Workflow, Deployment)
 
 ## [0.3.0] [![user](https://img.shields.io/badge/adamj335-181717.svg?style=flat&logo=github)](https://github.com/adamj335)

src/app/archbishop/archbishop.py

@@ -1,6 +1,7 @@
 import logging
 
 import requests
+import shortuuid
 from flask import Blueprint, Response
 from flask import request
 from slack_sdk.errors import SlackApiError
@@ -25,7 +26,10 @@ def hello():
     @archbishop.route("/scroll", methods=['POST'])
     def unroll():
         """ Handle slack command for scroll information """
-        group = redis.get_complex(request.form["team_id"], Group)
+        group_id = request.form["team_id"]
+        group = redis.get_complex(group_id, Group)
+        if group is None:
+            return Response("Cannot find group with id " + group_id, status=400)
         formatted_scroll = ""
         position = 0
         for record in group.scroll:
@@ -40,7 +44,10 @@ def unroll():
     def group(group_id):
         """ Get group information by id """
         log.debug(f"[get_group] Retrieving group for id [{group_id}]")
-        return Response(str(redis.get_complex(group_id, Group)), status=200)
+        group = redis.get_complex(group_id, Group)
+        if group is None:
+            return Response("Cannot find group with id " + group_id, status=400)
+        return Response(str(group), status=200)
 
     @archbishop.route(config.EVENT_PATH, methods=['POST'])
     def event():
@@ -80,7 +87,7 @@ def get_group(event):
             return group
         else:
             log.debug(f"[get_group] Creating new group with id [{group_id}]")
-            dummy_king = Player("", -1, 0)
+            dummy_king = Player("", -1, "", 0)
             group = Group(group_id, [], dummy_king, [])
             redis.set_complex(group_id, group)
             return group
@@ -94,7 +101,7 @@ def get_player(event, group):
             potential_player = [p for p in group.players if p.name == user]
 
             if not potential_player:
-                player = Player(user, 0, 0)
+                player = Player(user, 0, shortuuid.uuid(), 0)
                 group.players.append(player)
                 redis.set_complex(group.name, group)
                 log.debug(f"[get_player] [{user}] added to the system")

src/app/model/group/group.py

@@ -1,5 +1,6 @@
-import datetime
 from dataclasses import dataclass
+from datetime import datetime
+import shortuuid
 from src.app.model.base import Base
 from src.app.model.group.record import Record
 from src.app.model.group.player import Player
@@ -31,28 +32,56 @@ def from_dict(cls, dic):
         )
 
     def update_player(self, player):
+        """ Update player and handle streak reset if necessary """
         for index, p in enumerate(self.players):
             if p.name == player.name:
+                if player.streak == 0:
+                    player.streak_id = shortuuid.uuid()
                 self.players[index] = player
 
     def update_scroll(self, player):
-        """ Create new record """
-        timestamp = datetime.datetime.today().strftime('%d/%m/%Y')
-        new_record = Record(player.name, player.streak, timestamp)
-        self.scroll.append(new_record)
+        """ Update scroll if new streak is worthy """
+        if self.__is_empty_scroll():
+            self.scroll.append(player.get_record())
+            return
+
+        if self.__is_unworthy(player.streak):
+            return
 
-        # Sort and remove tail Record
-        self.scroll = sorted(self.scroll, key=lambda x: x.streak, reverse=True)
-        if len(self.scroll) > 3:
-            self.scroll.pop()
+        if self.__is_active(player.streak_id):
+            unsorted_scroll = [player.get_record() if x.streak_id == player.streak_id else x for x in self.scroll]
+            self.scroll = sorted(unsorted_scroll, key=lambda x: x.streak, reverse=True)
+
+        else:
+            self.scroll.append(player.get_record())
+            self.scroll = sorted(self.scroll, key=lambda x: x.streak, reverse=True)
+            if len(self.scroll) > 3:
+                self.scroll.pop()
 
     def crown(self, player):
         self.king = player
 
     def dethrone(self):
-        self.king = Player("", -1, 0)
+        self.king = Player("", -1, "", 0)
         non_zeros = [p for p in self.players if p.streak != 0]
-        if non_zeros is not list:
+        if len(non_zeros) == 0:
             return
         else:
             self.king = sorted(non_zeros, key=lambda x: x.streak, reverse=True)[0]
+
+    def __is_unworthy(self, new_streak):
+        """ Determine if streak is unworthy of scroll update """
+        sorted_scroll = sorted(self.scroll, key=lambda x: x.streak, reverse=False)
+        if new_streak < 2:
+            return True
+        else:
+            return len(self.scroll) == 3 and new_streak < sorted_scroll[0].streak
+
+    def __is_active(self, streak_id):
+        """ Determine if player streak is active in scroll by comparison with recorded streak ids """
+        matching_ids = [x for x in self.scroll if x.streak_id == streak_id]
+        return len(matching_ids) != 0
+
+    def __is_empty_scroll(self):
+        """ Determine if scroll is empty """
+        return len(self.scroll) == 0

src/app/model/group/group_spec.py

@@ -0,0 +1,137 @@
+from datetime import datetime
+
+from src.app.model.group.group import Group
+from src.app.model.group.player import Player
+from src.app.model.group.record import Record
+
+
+class GroupSpec:
+    group_name = 'group_name'
+
+    def should_not_update_scroll_if_new_streak_lower_than_the_lowest_record_streak(self):
+        """ Should not update scroll if new streak lower than the lowest record streak """
+        # Given
+        players = [Player('Adam', 4, '1', 1000), Player('Hayden', 3, '2', 1000), Player('James', 2, '3', 1000)]
+        scroll = [Record('Adam', 4, '1', 'today'), Record('Hayden', 3, '2', 'today'), Record('James', 2, '3', 'today')]
+        subject = Group(self.group_name, players, Player('Adam', 4, '1', 1000), scroll)
+
+        test = Player('Maciej', 1, '4', 1000)
+
+        # When
+        subject.update_scroll(test)
+
+        # Then : scroll is left untouched
+        assert subject.scroll is scroll
+
+    def should_update_scroll_if_new_streak_lower_than_the_lowest_record_streak_but_scroll_has_capacity(self):
+        """ Should update scroll if new streak lower than the lowest record streak but scroll has capacity"""
+        # Given
+        players = [Player('Adam', 4, '1', 1000), Player('Hayden', 3, '2', 1000)]
+        scroll = [Record('Adam', 4, '1', 'today'), Record('Hayden', 3, '2', 'today')]
+        subject = Group(self.group_name, players, Player('Adam', 4, '1', 1000), scroll)
+
+        test = Player('Maciej', 2, '4', 1000)
+        # Note :: streaks are defined as being >= 2
+
+        # When
+        subject.update_scroll(test)
+
+        # Then : test is added to the scroll
+        assert subject.scroll == [Record('Adam', 4, '1', 'today'), Record('Hayden', 3, '2', 'today'), test.get_record()]
+
+    def should_update_existing_record_if_player_streak_is_active_on_the_scroll(self):
+        """ Should update existing record if worthy player streak is active on the scroll """
+        # Given
+        players = [Player('Adam', 4, '1', 1000), Player('Hayden', 3, '2', 1000), Player('James', 2, '3', 1000)]
+        scroll = [Record('Adam', 4, '1', 'today'), Record('Hayden', 3, '2', 'today'), Record('James', 2, '3', 'today')]
+        subject = Group(self.group_name, players, Player('Adam', 4, '1', 1000), scroll)
+
+        test = Player('Adam', 5, '1', 1005)
+
+        # When
+        subject.update_scroll(test)
+
+        # Then : 'Adam' Record with streak id '1' should be updated
+        updated_record = [x for x in subject.scroll if x.streak_id == test.streak_id][0]
+        assert updated_record.streak is 5
+
+    def should_add_new_record_if_worthy_streak_is_not_active_on_the_scroll_and_scroll_below_capacity(self):
+        """ Should add new record if worthy streak is not active on the scroll and scroll below capacity """
+        # Given
+        players = [Player('Adam', 4, '1', 1000), Player('Hayden', 3, '2', 1000)]
+        scroll = [Record('Adam', 4, '1', 'today'), Record('Hayden', 3, '2', 'today')]
+        subject = Group(self.group_name, players, Player('Adam', 4, '1', 1000), scroll)
+
+        test = Player('Adam', 5, '3', 1005)
+
+        # When
+        subject.update_scroll(test)
+
+        # Then : 'Adam' Record w/streak id '1' should be preserved; second 'Adam' Record added w/streak id '2'
+        # Note : also expect scroll to be sorted upon update
+        expected_record = Record('Adam', 5, '3', datetime.today().strftime('%d/%m/%Y'))
+        expected_scroll = [expected_record, Record('Adam', 4, '1', 'today'), Record('Hayden', 3, '2', 'today')]
+        difference = [x for x in subject.scroll if x not in expected_scroll]
+        assert len(difference) is 0
+
+    def should_replace_an_old_record_if_worthy_streak_is_not_active_on_the_scroll_and_scroll_at_capacity(self):
+        """ Should replace an old record if worthy streak is not active on the scroll and scroll at capacity """
+        # Given
+        players = [Player('Adam', 4, '1', 1000), Player('Hayden', 3, '2', 1000), Player('James', 2, '3', 1000)]
+        scroll = [Record('Adam', 4, '1', 'today'), Record('Hayden', 3, '2', 'today'), Record('James', 2, '3', 'today')]
+        subject = Group(self.group_name, players, Player('Adam', 4, '1', 1000), scroll)
+
+        test = Player('Adam', 5, '4', 1005)
+
+        # When
+        subject.update_scroll(test)
+
+        # Then : 'Adam' Record w/streak id '1' should be preserved; second 'Adam' Record added w/streak id '2'
+        # Note : also expect scroll to be sorted upon update
+        expected_record = Record('Adam', 5, '4', datetime.today().strftime('%d/%m/%Y'))
+        expected_scroll = [expected_record, Record('Adam', 4, '1', 'today'), Record('Hayden', 3, '2', 'today')]
+        difference = [x for x in subject.scroll if x not in expected_scroll]
+        assert len(difference) is 0
+
+    def should_dethrone_king_by_crowning_next_highest_player(self):
+        """ Should dethrone king by crowning next highest player """
+        # Given
+        players = [Player('Adam', 4, '1', 1000), Player('Hayden', 3, '2', 1000), Player('James', 2, '3', 1000)]
+        subject = Group(self.group_name, players, Player('Adam', 4, '1', 1000), [])
+
+        test = Player('Adam', 0, '1', 1000)
+
+        # When
+        subject.update_player(test)
+        subject.dethrone()
+
+        # Then : King is Hayden
+        assert subject.king == Player('Hayden', 3, '2', 1000)
+
+    def should_reset_streak_id_when_player_has_streak_of_zero(self):
+        """ Should reset streak id when player has streak of zero """
+        # Given
+        players = [Player('Adam', 4, '1', 1000)]
+        subject = Group(self.group_name, players, Player('Adam', 4, '1', 1000), [])
+
+        test = Player('Adam', 0, '1', 1000)
+
+        # When
+        subject.update_player(test)
+
+        # Then : Adam should have new streak id
+        assert [x for x in subject.players if x.name == 'Adam'][0].streak_id != '1'
+
+    def should_not_reset_streak_id_when_player_has_non_zero_streak(self):
+        """ Should not reset streak id when player has non-zero streak """
+        # Given
+        players = [Player('Adam', 4, '1', 1000)]
+        subject = Group(self.group_name, players, Player('Adam', 4, '1', 1000), [])
+
+        test = Player('Adam', 5, '1', 1000)
+
+        # When
+        subject.update_player(test)
+
+        # Then : Adam should have new streak id
+        assert [x for x in subject.players if x.name == 'Adam'][0].streak_id == '1'

src/app/model/group/player.py

@@ -1,19 +1,26 @@
 from dataclasses import dataclass
-
+from datetime import datetime
 from src.app.model.base import Base
+from src.app.model.group.record import Record
 
 
 @dataclass
 class Player(Base):
     """ Tracked player information """
     name: str
     streak: int
+    streak_id: str
     score: int
 
     @classmethod
     def from_dict(cls, dic):
         return cls(
             name=dic["name"],
             streak=dic["streak"],
+            streak_id=dic["streak_id"],
             score=dic["score"]
         )
+
+    def get_record(self):
+        """ Convert a player into a record """
+        return Record(self.name, self.streak, self.streak_id, datetime.today().strftime('%d/%m/%Y'))

src/app/model/group/record.py

@@ -7,12 +7,14 @@ class Record(Base):
     """ Tracked Record for the Historic Streaks """
     name: str
     streak: int
+    streak_id: str
     date: str  # TODO :: actual date
 
     @classmethod
     def from_dict(cls, dic):
         return cls(
             name=dic["name"],
             streak=dic["streak"],
+            streak_id=dic["streak_id"],
             date=dic["date"]
         )

src/app/requirements.txt

@@ -20,3 +20,4 @@ coverage-badge
 pytest-cov
 genbadge
 defusedxml
+shortuuid