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

[Add] TODOリクエスト機能の追加 #3

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open
2 changes: 1 addition & 1 deletion cogs/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ async def do_eval(self, ctx, body):
value = stdout.getvalue()
embed = discord.Embed(
title="Error",
descriription=f"エラーが発生しました。コードを確認してください。\ntraceback:```py\n{value}{traceback.format_exc()}\n```"
description=f"エラーが発生しました。コードを確認してください。\ntraceback:```py\n{value}{traceback.format_exc()}\n```"
)
await ctx.send(embed=embed)
else:
Expand Down
157 changes: 125 additions & 32 deletions cogs/todo.py
Original file line number Diff line number Diff line change
@@ -1,35 +1,51 @@
import discord
from discord.ext import commands
import json
import numpy
import textwrap

REACTIONS = ["👍", "👎"]
Copy link

Choose a reason for hiding this comment

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

REACTIONS[0] が up で REACTIONS[1] が down なのは非自明。どっちがどっちかすぐ間違えちゃいそう。

REACTION_AGREEREACTION_DISAGREE だとか、
REACTION_YESREACTION_NO みたいな感じで、
役割を明らかにした名前にするのが良い。

Copy link

Choose a reason for hiding this comment

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

あ、リアクションの集合を表す REACTIONS も必要みたいね。
そしたら

REACTIONS = [REACTION_YES, REACTION_NO]

みたいにするのがいい。

Copy link
Owner Author

Choose a reason for hiding this comment

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

あーなるほど!
そうさせていただきます!



class TODOCog(commands.Cog):
def __init__(self, bot):
self.bot = bot

def _make_request(self, ctx, user_send_to, todo):
req_id = numpy.base_repr(ctx.message.id, 36)
data = {
"author": ctx.author.id,
"to": user_send_to.id,
"content": todo
}
self.bot.data["request"][req_id] = data
self.bot.save_data()
return req_id

@commands.command(name="help")
async def _help(self, ctx):
msg = """
>>> TODO:
- やること
msg = """\
>>> TODO:
- やること

のようなフォーマットのメッセージを書くと認識して記憶してくれる、ただそれだけのボットです。
ハイフンの前のスペースが何個でも認識してくれるようになりました。
のようなフォーマットのメッセージを書くと認識して記憶してくれる、ただそれだけのボットです。
ハイフンの前のスペースが何個でも認識してくれるようになりました。

`todo!list` でTODOのリストが見れます。(alias: `todo!l`)
`todo!delete TODO番号` でTODOを削除できます。(alias: `todo!d`)
`todo!list` でTODOのリストが見れます。(alias: `todo!l`)
`todo!delete TODO番号` でTODOを削除できます。(alias: `todo!d`)

**招待リンク**
https://discord.com/api/oauth2/authorize?client_id=716294656987758702&permissions=67648&scope=bot
(GitHubにあげるのめんどいのでソースコードは)ないです。"""
return await ctx.send(msg)
**招待リンク**
https://discord.com/api/oauth2/authorize?client_id=716294656987758702&permissions=67648&scope=bot
**ソースコード**
https://github.com/daima3629/TODObot
"""
return await ctx.send(textwrap.dedent(msg))

@commands.Cog.listener()
async def on_message(self, message):
if not message.content.startswith("TODO:"):
return

s = message.content.split("\n")
s = message.content.splitlines()
if len(s) < 2:
return
todos = s[1:]
Expand All @@ -45,22 +61,23 @@ async def on_message(self, message):
msg += f"{todo}\n"
msg += "\nTODOに追加しますか?"
msg = await message.channel.send(msg)
Copy link

Choose a reason for hiding this comment

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

message(受信したメッセージのオブジェクト)
msg(確認のために送信するメッセージのテキスト)
再代入して
msg(確認のために送信したメッセージのオブジェクト)
ってなんか凄く追いづらい気がする

変数名を
confirm_message_text
confirm_message
とかにすると分かりやすい

await msg.add_reaction("👍")
await msg.add_reaction("👎")
def check(reac, user):
await msg.add_reaction(REACTIONS[0])
await msg.add_reaction(REACTIONS[1])
Copy link

Choose a reason for hiding this comment

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

REACTIONS の中身全部 for 文とかで add するようにした方が間違い無さそう。
(例えばリアクションの種類を増やしたくなった時とかに、こういう箇所での追加漏れを防ぎやすい。)


def check(reaction, user):
if not user == message.author:
return False
if not str(reac.emoji) in ["👍", "👎"]:
if not str(reaction.emoji) in REACTIONS:
return False
return True
reac, _ = await self.bot.wait_for("reaction_add", check=check)
if str(reac.emoji) == "👍":
if not self.bot.data.get(str(message.author.id)):
self.bot.data[str(message.author.id)] = []

reaction, _ = await self.bot.wait_for("reaction_add", check=check)
if str(reaction.emoji) == REACTIONS[0]:
if not self.bot.data["todo"].get(str(message.author.id)):
Copy link

Choose a reason for hiding this comment

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

Suggested change
if not self.bot.data["todo"].get(str(message.author.id)):
if str(message.author.id) not in self.bot.data["todo"]:

get を使ってると、ぱっと見た時何書いてるかわからなかった

Copy link

Choose a reason for hiding this comment

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

has_key の代用で key in dict が出来るので、そちらのほうが良いと思われる

self.bot.data["todo"][str(message.author.id)] = []

for todo in todos:
self.bot.data[str(message.author.id)].append(todo)
self.bot.data["todo"][str(message.author.id)].append(todo)
self.bot.save_data()
await message.channel.send(">>> 正常にTODOに追加されました。\n一覧を見るには`todo!list`")
await msg.delete()
Expand All @@ -69,38 +86,114 @@ def check(reac, user):

@commands.command(name="list", aliases=["l"])
async def _list(self, ctx):
l = self.bot.data.get(str(ctx.author.id))
todo_list = self.bot.data["todo"].get(str(ctx.author.id))
todos = ""
if not l:
if not todo_list:
return await ctx.send(f">>> {ctx.author.mention}のTODOリスト\n\nNone")

for i, todo in enumerate(l):
for i, todo in enumerate(todo_list):
todos += f"{i}: {todo}" + "\n"
Copy link

Choose a reason for hiding this comment

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

todos の追加の仕方がスマートじゃない気がする
enumerate を使ってるのは良い
空文字用意 → 必要なものと改行を結合 → 最後の改行を消す
って処理するくらいなら、リスト内包表記使って join するのが良いと思う

todos = todos.rstrip("\n")
msg = f">>> {ctx.author.mention}のTODOリスト\n\n" + todos
return await ctx.send(msg)

@commands.command(aliases=["d"])
async def delete(self, ctx, num: int):
l = self.bot.data.get(str(ctx.author.id))
if not l:
todo_list = self.bot.data["todo"].get(str(ctx.author.id))
if not todo_list:
return await ctx.send("> そのTODOはありません。`todo!list`で確認してください。")
try:
deleted_todo = self.bot.data[str(ctx.author.id)][num]
del self.bot.data[str(ctx.author.id)][num]
deleted_todo = self.bot.data["todo"][str(ctx.author.id)][num]
del self.bot.data["todo"][str(ctx.author.id)][num]
Comment on lines +110 to +111
Copy link

Choose a reason for hiding this comment

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

Suggested change
deleted_todo = self.bot.data["todo"][str(ctx.author.id)][num]
del self.bot.data["todo"][str(ctx.author.id)][num]
deleted_todo = todo_list[num]
del todo_list[num]

上で todo_list 定義してるんだし、そのまま使えば良さそう

Copy link
Owner Author

Choose a reason for hiding this comment

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

todo_list内のデータを削除すればself.bot.data内のデータも一緒に削除できるんですか?

Copy link
Owner Author

Choose a reason for hiding this comment

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

あ、検証してみたところ消されました
Pythonの参照渡しというのはdict.getでも働くんですね...
ありがとうございます!

Copy link

Choose a reason for hiding this comment

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

Python は全部「値渡し」だよ。「Python の引数は参照渡し」ってデマ書いてるサイト、めちゃくちゃ多いけど。

cf.
https://qiita.com/ur_kinsk/items/949dabe975bdc1affb82
https://qiita.com/raccy/items/d4c5e7995a8fc90109ee

このへん一度ちゃんと理解したら、検証しなくても色々一発で予想できるようになると思う。

except IndexError:
return await ctx.send("> そのTODOはありません。`todo!list`で確認してください。")
self.bot.save_data()
return await ctx.send(f"> TODO`{deleted_todo}`の削除に成功しました。")

@delete.error
async def error_delete(self, ctx, err):
if isinstance(err, commands.errors.BadArgument):
return await ctx.send("> 引数は整数で指定してください。")
if isinstance(err, commands.errors.MissingRequiredArgument):
return await ctx.send(">>> TODOの番号を引数で指定してください。\n番号は`todo!list`で確認できます。")

@commands.group(aliases=["req"])
async def request(self, ctx):
if not ctx.invoked_subcommand:
return await ctx.send("> サブコマンドを指定してください。")

@request.command(aliases=["c"])
async def create(self, ctx, member: discord.Member, *, todo):
if not todo:
return await ctx.send("> 引数に送りたいTODOを指定してください。")
text = f"""\
>>> `{member}`さんにtodoリクエストを送ります。
内容:
・{todo}

よろしいですか?
"""
msg = await ctx.send(textwrap.dedent(text))
await msg.add_reaction(REACTIONS[0])
await msg.add_reaction(REACTIONS[1])

def check(reaction, user):
if not user == ctx.author: return
if not reaction.message == msg: return
if not str(reaction.emoji) in REACTIONS: return
return True

reaction, _ = await self.bot.wait_for("reaction_add", check=check)
if str(reaction.emoji) == REACTIONS[1]:
await msg.delete()
return await ctx.send("> todoリクエストをキャンセルしました。", delete_after=5)

req_id = self._make_request(ctx, member, todo)
dm_msg = f"""\
>>> {ctx.author.mention}さんからTODOリクエストが届きました。

内容:
・{todo}

リクエストID: `{req_id}`

このリクエストを承認する場合は`todo!request approve {req_id}`
拒否する場合は`todo!request deny {req_id}`
とコマンドを実行してください。
"""
try:
await member.send(textwrap.dedent(dm_msg))
except discord.Forbidden:
await ctx.send(f"{member.mention}\n" + textwrap.dedent(dm_msg))
Comment on lines +147 to +162

Choose a reason for hiding this comment

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

ここIDだけではなく(TODOを追加するところと同じように)リアクションでできたら良さそうかな〜と思いました

Copy link
Owner Author

Choose a reason for hiding this comment

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

そこどうしようか迷ってたんですが、やっぱリアクション確認入れたほうがいいですね
ありがとうございます!


result = f"""
>>> リクエストの送信に成功しました。

リクエストID: `{req_id}`
"""
return await ctx.send(textwrap.dedent(result))

@request.command(aliases=["a"])
async def approve(self, ctx, req_id: str):
req_todo = self.bot.data["request"].get(req_id)
if not req_id:
Copy link

Choose a reason for hiding this comment

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

Suggested change
if not req_id:
if not req_todo:

ミス?

Copy link

Choose a reason for hiding this comment

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

deny の方と統一されてないので、何が正しいかわからなくなった

return await ctx.send("> そのIDのリクエストは存在しません。もう一度確認してください。")

self.bot.data["todo"][str(ctx.author.id)].append(req_todo.content)
del self.bot.data["request"][req_id]
self.bot.save_data()
return await ctx.send(f"> リクエストを承認しました。\n\n追加されたTODO:\n・{req_todo.content}")

@request.command(aliases=["d"])
async def deny(self, ctx, req_id: str):
if not req_id:
return await ctx.send("> そのIDのリクエストは存在しません。もう一度確認してください。")

del self.bot.data["request"][req_id]
self.bot.save_data()
return await ctx.send("> リクエストを拒否しました。")


def setup(bot):
bot.add_cog(TODOCog(bot))
print("todocog loaded")
print("TODOCog loaded")