Skip to content
This repository was archived by the owner on Oct 27, 2020. It is now read-only.

MECHA-253: Implement !reopen without refreshing all cases #260

Open
MHajoha wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

MECHA-253: Implement !reopen without refreshing all cases #260

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3401f50
Modify !reopen to not reindex the whole board
MHajoha Jan 27, 2018
111cb9d
Update saved rats while were at it and convert data to v1 before maki…
MHajoha Jan 27, 2018
3487f3f
Pluck out broad except clause and give addNamesFromV2Response a docst…
MHajoha Feb 1, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions sopel_modules/rat_board.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1648,9 +1648,19 @@ def cmd_reopen(bot, trigger, id):
"""
try:
result = callapi(bot, 'PUT', data={'status': 'open'}, uri='/rescues/' + str(id), triggernick=str(trigger.nick))
refresh_cases(bot, force=True)
updateBoardIndexes(bot)
bot.say('Reopened case. Cases refreshed, care for your case numbers!')
try:
addNamesFromV2Response(result['included'])
except:
pass
Copy link
Contributor

@theunkn0wn1 theunkn0wn1 Jan 31, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not use a bare except block.

Further, why are we catching bare exceptions and not handling them?

Copy link
Member Author

@MHajoha MHajoha Jan 31, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep I agree that this is terribly bad style of code. This is just how this is done in the rest of the code and I couldn't (be bothered to) figure out what addNamesFromV2Response actually throws.

Copy link
Contributor

@theunkn0wn1 theunkn0wn1 Jan 31, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what i can tell it shouldn't be raising an exception. and if it does methinks we would like to know about it. Catch/pass is awful code style. I would prefer if mecha raised an error rather than silence it, that's pretty much the only indication we get if something is broken.

There are a lot of things you may discover in mecha2's codebase that are just plain bad practice.
For now it would be appreciated that any method you introduce/modify you make attempt to clean up.

We will just have to maintain mecha2 until mecha3 is ready to go. (which may be a while)

Copy link
Member Author

@MHajoha MHajoha Feb 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah you're right, I'll have a look at fixing that except clause tomorrow.

Copy link
Member Author

@MHajoha MHajoha Feb 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, is Mecha3 already on its way to becoming a thing? As in a complete rewrite against pydle or similar?


rescue = Rescue.load(convertV2DataToV1(result['data'])[0])
bot.memory['ratbot']['board'].add(rescue)

with rescue.change():
rescue.data.update({"boardIndex": rescue.boardindex})

save_case_later(bot, rescue, forceFull=True)
bot.say('Reopened case {case.client_name}. It is now #{case.boardIndex}.'.format(case=rescue))
except ratlib.api.http.APIError:
# print('[RatBoard] apierror.')
bot.reply('id ' + str(id) + ' does not exist or other API Error.')
Expand Down