Bookmark Command

[Nxllpointer]

Closes (#570)

• Implement /bookmarks add [note] command
• Implement /bookmarks list command
• Implement /bookmarks remove command
• Limit note size
• Add spam protection
• Delete bookmarks of users that left the server after a week

Screenshots:
(2 entries a page for demonstrating purposes)

[marko-radosavljevic]

Thank you for your contribution, and welcome to the project! ☺️ ❤️

[Nxllpointer]

Thank you!

[marko-radosavljevic]

Ran the checks for you, so you can be aware of issues early on.

Feel free to ping me whenever you want me to run them again. ^^

[Nxllpointer]

Ah ok great. Im gonna fix them real quick

[Nxllpointer]

I made everything sonarlint compliant and the tests passed locally. Should be fine now. I dont neccesairily agree with the switch statement changes but hey.

[Taz03]

isn't this command is supposed to work in dms too?? and if not u can set the command visibility to guild no need to handle that explicitly

[Zabuzard]

isn't this command is supposed to work in dms too?

that would be ideal yes. should be as simple as making it a GLOBAL command instead of GUILD.

[Nxllpointer]

Yes it does already work in DMs. I also handled trying to add a bookmark in a DM

[Nxllpointer]

Ok. I have now added sorting by activity and thats all i wanted to add. The UI still looks the same.

[Taz03]

there are extra empty lines in ur code at places like, between methods, between fields, etc. it'd be nice if u can look for them and fix it

[Nxllpointer]

there are extra empty lines in ur code at places like, between methods, between fields, etc. it'd be nice if u can look for them and fix it

I did this on purpose. It makes the code more organized in my opinion. Like having an extra newline between static things, grouping methods and stuff like that. I think it makes it much more readable.

[Tais993]

I'll do a full review tomorrow, but just submitting this for now

[Zabuzard]

mh. do it like this:

• limit the bookmarks per user to 500.
• delete bookmarks for an user if they leave the guild
• delete bookmarks for threads that are deleted (not archived, deleted)
• write a warning message in modauditlog if the total amount of bookmarks exceeds 1 million
• totally block creation of new bookmarks for everyone once the total amount exceeds 1_100_000 bookmarks

disk space isnt really a concern for us. we just have to prevent that someone exploits it and spams us to death (gigabytes, terrabytes)

this should do the trick and is easier than expiration

and better ux, since the user doesnt have to keep their bookmarks alive

[Zabuzard]

for better ux, i think the pagination should display 10 embeds, not just one.

[Taz03]

delete bookmarks for an user if they leave the guild

no, y?

[Taz03]

for better ux, i think the pagination should display 10 embeds, not just one.

then also state a solution for delete thing rn user can scroll through each bookmark and delete one they want by clicking the delete button, how'll this work with 10 embeds?

[Zabuzard]

delete bookmarks for an user if they leave the guild

no, y?

whats the point of keeping bookmarks for an user who is gone? they cant use the bookmarks anymore since they are not part of the server. they cant click the channel links, the channels are effectively dead for them. we need to do housekeeping, otherwise the database keeps growing indefinitely.

[Zabuzard]

for better ux, i think the pagination should display 10 embeds, not just one.

then also state a solution for delete thing rn user can scroll through each bookmark and delete one they want by clicking the delete button, how'll this work with 10 embeds?

this is a good point. in general, i am voting for a /bookmark delete command anyways, it has the better and more straightforward entry-ux. if an user wants to delete a bookmark, they will first look for such a command and might miss the fact that /bookmark list can be used for that. in fact, if we keep the current way of deleting, i would even rename list to manage, to make this clearer.

that said, i am unsure about a really good ux for a /bookmark delete command. realistically, one would want to enter the title of the bookmarked thread (with auto-complete fuzzy-matching, such as we just did for TagCommand).

personally, i think making the delete-flow a bit more cumbersome is worth the price. the list flow is used far more frequently and will be one of the main use cases for this. it just has to be good. and the ux, right now, if u have lots of bookmarks, is just super annoying.

like, u need a decent way of getting a quick overview of all ur 100 bookmarks. and clicking "next" 100 times certainly isnt.

[Nxllpointer]

but deleting bookmarks with commands is also really annoying.
Bookmarks expiring after a few days would help with that ;)

[Nxllpointer]

Days since messing up my commit history: 0

[Taz03]

make a package coz u have multiple files

[Nxllpointer]

Zabuzards rework has been merged dd85945

[Nxllpointer]

Everything resolved and no conflicts anymore. We should be able to merge now