-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Drag challenge tasks #12204
Drag challenge tasks #12204
Conversation
In my opinion I think that's a good change. When you join a challenge, you're likely to want to look at / adjust / move your new tasks straight away, which is easier if they start at the top. I've labelled this as "notify helpers" so that if this particular change does go live, I'll remember to tell the Socialites guild, Challenge Leaders guild, and wiki editors about it. |
thanks @alecbrick for picking this up again ! I'll review it in the next few days, but I'm wondering if this comment #12021 (comment) got addressed in this version (the fact that the API doesn't order the user tasks but it is done on the client and we should probably do the same for the challenge and group ones) |
Hi @alecbrick, any update on my last comment? I'm happy to finish this if you don't have time, just let me know :) |
Sorry for the late response! No, we're still ordering challenge and group tasks on the server here - haven't changed anything. |
@alecbrick okay, as I said in a previous comment, I understand it might not be the best option but for the time being I prefer if the ordering remains on the client side like for user tasks. Let me know if you want to make the changes or if you can't that's totally fine I'll take over |
@paglias If you're able to take this over, I think that would be fine! |
I'm back! @paglias I would really prefer not to add the client-side ordering in this PR. The idea that a Column is connected to a User is heavily baked-in, and a lot of the ordering logic for the user tasks revolves around that. If we were to add client-side ordering for challenges and groups, it would involve a lot of refactoring that doesn't really fit with this PR. If client-side ordering absolutely must happen here, that's fine, but IMO it's a separate issue |
Just following up - @paglias, are you able to review this? |
Hi Alec, unfortunately I will not be able to help with the review of this PR. Pinging @SabreCat |
Thanks so much @alecbrick, this is fantastic! This will make a lot of users happy. 😸 Welcome to Blacksmith contributor tier 3! |
Fixes #1703
I worked on this one previously (in PR #12021), but work got pretty time-consuming when I only had a few minor edits left. Sorry about that - I think this is ready for review!
Changes
Currently, challenge and group tasks can't be dragged and reordered. This change allows them to be reordered by anyone who can create or modify tasks.
However, one of the issues with allowing challenge and group tasks to be reordered is that challenges and groups don't respect the
tasksOrder
object. As such, this PR also ensures that the tasks are always ordered according totasksOrder
, which is done by ordering the tasks in the GET /tasks/user endpoint.One thing to note is that in the front end, challenges and groups currently add tasks at the end of their lists, while user tasks are added at the beginning of their lists. However, this all occurs in one place in the backend, where the tasks are added to the beginning of their
tasksOrder
lists. In order to unify these, I've changed challenges and groups so that new tasks are added at the beginning of their lists, rather than at the end. Let me know if that's not something we want!Additionally, I've refactored some of the code in the Tasks API that checks whether a user can modify a task.
UUID: 3c2b1d99-c6f0-4e6e-a67a-bd8e023cfc7e