TJ-Bot
TJ-Bot copied to clipboard
Together-Java
Bookmark Command
Resolves #570
- [x] Implement
/bookmarks add [note]command - [x] Implement
/bookmarks listcommand - [ ] Implement
/bookmarks removecommand
Screenshots:
Thank you for your contribution, and welcome to the project! :relaxed: :heart:
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. ^^
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.
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
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.
Yes it does already work in DMs. I also handled trying to add a bookmark in a DM
Ok. I have now added sorting by activity and thats all i wanted to add. The UI still looks the same.
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
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.
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?
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.
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.
but deleting bookmarks with commands is also really annoying. Bookmarks expiring after a few days would help with that ;)
whats the point of keeping bookmarks for an user who is gone?
if someone left the server by mistake and join just after that they'll loose all their bookmarks, also iirc the current system automatically removes the bookmarks after 3 days so if they actually left the server the bookmark will be gone after 3 days automatically
How about adding a /bookmarks delete command that gives you a message with a selection menu which to delete, like you can do with your helper roles?
How about adding a /bookmarks delete command that gives you a message with a selection menu which to delete, like you can do with your helper roles?
i like the selection menu suggestion. note though that selection menus are limited. so it needs a page parameter as well. such as /boomarks delete show-page: 3
if someone left the server by mistake and join just after that they'll loose all their bookmarks
then we delay the deletion for a week. but we should still cleanup bookmarks from dead users. for example by adding a column to the database delete_at TIMESTAMP with a cleanup routine. adding such a column is probably a good idea anyways, since its generic enough to also be used in a different context.
Bookmarks expiring after a few days would help with that ;)
for me, thats a no-go. the point of a bookmark is to have, well, a bookmark. i want to be able to view that again in 1 year.
Ok so now lets recap how this should be done.
Noone had something to say about the bookmark add command so it should stay the same. For listing bookmarks i would suggest still using a pagination system but with 10 bookmarks per page. The user can then choose in a SelectionMenu (25 entries max) which bookmarks to delete.
React with :+1: if you agree
@Nxllpointer sounds good to me. but the deletion part needs some sort of pagination system (as explained above) as well. users could have hundreds of bookmarks and it has to be able to handle that.
wait so the list command still do the delete stuff? i'd say have a separate command for deleting, how it'll work is u get an embed with pagination (25 bookmarks per page, as embed fields) the fields will just be the tag nothing else we don't need that info here list is for that purpose and we'll have a selection menu to select and then click the delete button to delete
Ok. So I did a full rewrite of this feature. Spam protection and the note size limit will be added later today. I had to force push this because when starting this feature I accidentially based my branch on the modmail feature branch.
Ok. So I did a full rewrite of this feature. Spam protection and the note size limit will be added later today. I had to force push this because when starting this feature I accidentially based my branch on the modmail feature branch.
Cool stuff. Could you add pictures of all the features? Easier to review with that :) For example the delete stuff isnt on the picture yet.
y the selection in the select menu says "delete bookmark n"?? y not just its title
Ok. My todo list is now complete. I would be very happy to get feedback on this and finally being done with this feature.