DiscordPHP icon indicating copy to clipboard operation
DiscordPHP copied to clipboard

Add lock/unlock method to Thread class

Open BaeFell opened this issue 1 year ago • 8 comments

Adds ability to lock/unlock threads using the Thread class.

BaeFell avatar Jul 06 '24 22:07 BaeFell

Not entirely sure how I ended up running composer run-script cs and it had quite a few changes to make.

BaeFell avatar Jul 06 '24 22:07 BaeFell

Other maintainers have argued that functions like this, which aren't directly documented by Discord, make the overall library harder to maintain. My primary question when reviewing this PR (and the Thread class in general) is: why doesn't the Channel class have a similar rename method using the patch endpoint, as the Thread class does? While I don't share the opinion that such functions complicate maintenance, it does suggest that these features might be better implemented as a shared trait among similar classes. I believe this PR is appropriate because it addresses a thread-specific issue (locking and unlocking) that doesn't apply to other contexts, so unless someone objects I am 100% for merging this.

valzargaming avatar Jul 06 '24 22:07 valzargaming

I'm with you having functions like this or common ones shared as a class trait is a good thing. As long as it's possible to do something and it's not completely unintended like modals having dropdowns, then having that functionality helps everyone.

BaeFell avatar Jul 07 '24 09:07 BaeFell

I messed up my PR/commit 🤦 My changes to the CommandBuilder were meant to go separate. I'm awful with Git, any advice would be appreciated to solve this 🙏

BaeFell avatar Jul 11 '24 22:07 BaeFell

I messed up my PR/commit 🤦 My changes to the CommandBuilder were meant to go separate. I'm awful with Git, any advice would be appreciated to solve this 🙏

Undo your command builder changes here and re-push. Afterwards, checkout back to the main branch of your fork using git checkout master and then create a new branch for your command builder changes with git checkout -b fix/commandbuilder-context-description and re-commit your command builder changes.

Log1x avatar Jul 11 '24 22:07 Log1x

I messed up my PR/commit 🤦 My changes to the CommandBuilder were meant to go separate. I'm awful with Git, any advice would be appreciated to solve this 🙏

Undo your command builder changes here and re-push. Afterwards, checkout back to the main branch of your fork using git checkout master and then create a new branch for your command builder changes with git checkout -b fix/commandbuilder-context-description and re-commit your command builder changes.

I'll give that a go tomorrow. Thank you!

BaeFell avatar Jul 11 '24 22:07 BaeFell

I'll give that a go tomorrow. Thank you!

I didn't realize you did this PR off of master – what I would do is close this PR and re-fork the repo ensuring you checkout a branch before doing your changes such as git checkout -b enhance/lock-unlock-thread – it'd also allow you to do your changes without running linting since as you noted it had quite a few unrelated changes.

Log1x avatar Jul 11 '24 23:07 Log1x

You should just create a new branch in your repository then cherry pick the latest commit. And then create new PR

Other maintainers have argued that functions like this, which aren't directly documented by Discord, make the overall library harder to maintain. My primary question when reviewing this PR (and the Thread class in general) is: why doesn't the Channel class have a similar rename method using the patch endpoint, as the Thread class does? While I don't share the opinion that such functions complicate maintenance, it does suggest that these features might be better implemented as a shared trait among similar classes. I believe this PR is appropriate because it addresses a thread-specific issue (locking and unlocking) that doesn't apply to other contexts, so unless someone objects I am 100% for merging this.

For case in Thread, there exists method archive() and unarchive() method, due to the fact that it can be done outside the conditions for threads->save() i,e. different permission/being owner of the thread or is using different endpoint/method.

Locked and unlocked did not have its own method because at that time you can just do $thread->lock = true then threads->save($thread)

SQKo avatar Jul 21 '24 12:07 SQKo