From 915b439e07c5b4aaef754f5f41360d862149a3fc Mon Sep 17 00:00:00 2001 From: Carl Worth Date: Fri, 8 Jan 2021 23:04:55 -0800 Subject: [PATCH] Don't re-set channel and sheet name or channel topic to the same value as before The puzzle_update_channel_and_sheet function is modified here to accept an old_puzzle argument in addition to the puzzle argument. By using this it can decide whether there's any change in the sheet or channel data before setting it. This optimization avoids Slack reporting things like: Turbot renamed the channel from "hunt-puzzle" to "hunt-puzzle" Which is obviously just noise. --- turbot/interaction.py | 27 +++++++++---- turbot/puzzle.py | 93 +++++++++++++++++++++++++++++-------------- 2 files changed, 83 insertions(+), 37 deletions(-) diff --git a/turbot/interaction.py b/turbot/interaction.py index 0b2f30e..3254f24 100644 --- a/turbot/interaction.py +++ b/turbot/interaction.py @@ -229,13 +229,18 @@ def edit_puzzle_submission(turb, payload, metadata): } ) + # Get old puzzle from the database (to determine what's changed) + old_puzzle = find_puzzle_for_puzzle_id(turb, + puzzle['hunt_id'], + puzzle['puzzle_id']) + # Update the puzzle in the database turb.table.put_item(Item=puzzle) # We need to set the channel topic if any of puzzle name, url, # state, status, or solution, has changed. Let's just do that # unconditionally here. - puzzle_update_channel_and_sheet(turb, puzzle) + puzzle_update_channel_and_sheet(turb, puzzle, old_puzzle=old_puzzle) return lambda_ok @@ -682,17 +687,20 @@ def state(turb, body, args): channel_id = body['channel_id'][0] - puzzle = puzzle_for_channel(turb, channel_id) + old_puzzle = puzzle_for_channel(turb, channel_id) - if not puzzle: + if not old_puzzle: return bot_reply( "Sorry, the /state command only works in a puzzle channel") - # Set the state field in the database + # Make a copy of the puzzle object + puzzle = old_puzzle.copy() + + # Update the puzzle in the database puzzle['state'] = args turb.table.put_item(Item=puzzle) - puzzle_update_channel_and_sheet(turb, puzzle) + puzzle_update_channel_and_sheet(turb, puzzle, old_puzzle=old_puzzle) return lambda_ok @@ -706,15 +714,18 @@ def solved(turb, body, args): channel_id = body['channel_id'][0] user_name = body['user_name'][0] - puzzle = puzzle_for_channel(turb, channel_id) + old_puzzle = puzzle_for_channel(turb, channel_id) - if not puzzle: + if not old_puzzle: return bot_reply("Sorry, this is not a puzzle channel.") if not args: return bot_reply( "Error, no solution provided. Usage: `/solved SOLUTION HERE`") + # Make a copy of the puzzle object + puzzle = old_puzzle.copy() + # Set the status and solution fields in the database puzzle['status'] = 'solved' puzzle['solution'].append(args) @@ -737,7 +748,7 @@ def solved(turb, body, args): ) # And update the puzzle's description - puzzle_update_channel_and_sheet(turb, puzzle) + puzzle_update_channel_and_sheet(turb, puzzle, old_puzzle=old_puzzle) return lambda_ok diff --git a/turbot/puzzle.py b/turbot/puzzle.py index 1a36bfb..f0456c6 100644 --- a/turbot/puzzle.py +++ b/turbot/puzzle.py @@ -153,51 +153,38 @@ def puzzle_matches_all(puzzle, patterns): def puzzle_id_from_name(name): return re.sub(r'[^a-zA-Z0-9_]', '', name).lower() -def puzzle_update_channel_and_sheet(turb, puzzle): - - channel_id = puzzle['channel_id'] - name = puzzle['name'] - url = puzzle.get('url', None) - sheet_url = puzzle.get('sheet_url', None) - state = puzzle.get('state', None) - status = puzzle['status'] +def puzzle_channel_topic(puzzle): + """Compute the channel topic for a puzzle""" topic = '' - if status == 'solved': + if puzzle['status'] == 'solved': topic += "SOLVED: `{}` ".format('`, `'.join(puzzle['solution'])) - topic += name + topic += puzzle['name'] links = [] + + url = puzzle.get('url', None) if url: links.append("<{}|Puzzle>".format(url)) + + sheet_url = puzzle.get('sheet_url', None) if sheet_url: links.append("<{}|Sheet>".format(sheet_url)) if len(links): topic += "({})".format(', '.join(links)) + state = puzzle.get('state', None) if state: topic += " {}".format(state) - # Slack only allows 250 characters for a topic - if len(topic) > 250: - topic = topic[:247] + "..." - - turb.slack_client.conversations_setTopic(channel=channel_id, - topic=topic) + return topic - # Rename the sheet to include indication of solved/solution status - sheet_name = puzzle['name'] - if puzzle['status'] == 'solved': - sheet_name += " - Solved {}".format(", ".join(puzzle['solution'])) +def puzzle_channel_name(puzzle): + """Compute the channel name for a puzzle""" - turbot.sheets.renameSheet(turb, puzzle['sheet_url'], sheet_name) - - # Finally, rename the Slack channel to reflect the latest name and - # the solved status - # # Note: We don't use puzzle['hunt_id'] here because we're keeping # that as a persistent identifier in the database. Instead we # create a new ID-like identifier from the current name. @@ -205,10 +192,58 @@ def puzzle_update_channel_and_sheet(turb, puzzle): puzzle['hunt_id'], puzzle_id_from_name(puzzle['name']) ) + if puzzle['status'] == 'solved': channel_name += "-solved" - turb.slack_client.conversations_rename( - channel=puzzle['channel_id'], - name=channel_name - ) + return channel_name + +def puzzle_sheet_name(puzzle): + """Compute the sheet name for a puzzle""" + + sheet_name = puzzle['name'] + if puzzle['status'] == 'solved': + sheet_name += " - Solved {}".format(", ".join(puzzle['solution'])) + + return sheet_name + +def puzzle_update_channel_and_sheet(turb, puzzle, old_puzzle=None): + + channel_id = puzzle['channel_id'] + + # Compute the channel topic and set it if it has changed + channel_topic = puzzle_channel_topic(puzzle) + + old_channel_topic = None + if old_puzzle: + old_channel_topic = puzzle_channel_topic(old_puzzle) + + if channel_topic != old_channel_topic: + # Slack only allows 250 characters for a topic + if len(channel_topic) > 250: + channel_topic = channel_topic[:247] + "..." + turb.slack_client.conversations_setTopic(channel=channel_id, + topic=channel_topic) + + # Compute the sheet name and set it if it has changed + sheet_name = puzzle_sheet_name(puzzle) + + old_sheet_name = None + if old_puzzle: + old_sheet_name = puzzle_sheet_name(old_puzzle) + + if sheet_name != old_sheet_name: + turbot.sheets.renameSheet(turb, puzzle['sheet_url'], sheet_name) + + # Compute the Slack channel name and set it if it has changed + channel_name = puzzle_channel_name(puzzle) + + old_channel_name = None + if old_puzzle: + old_channel_name = puzzle_channel_name(old_puzzle) + + if channel_name != old_channel_name: + turb.slack_client.conversations_rename( + channel=channel_id, + name=channel_name + ) -- 2.43.0