Starter issues solving - reply message + change bookmark category + space for usernames #1790
Conversation
done: creation of edit_modal.html and user interface in the bookmarks page. missing: it does not execute the request.method==POST part in view.py -> change_bookmark_category
improvements to be made: show error message inside edit modal in case the category already exists
quimmrc
changed the title
quimmrc
changed the title
There was a problem hiding this comment.
Hi! Thanks @quimmrc. I think you did a great job at finding a solution for the edit bookmark problem, although you ended up implementing some sort of custom solution and there are already ways to do this "form in modal" idea in Freesound. In the comments I left pointers to example code that show how you should do it. It should be easier at the end :)
Please have a look and let's discuss this in person if you don't understand it. Thanks a lot!!!
| validators=[RegexValidator(r'^[\w .+-]+$')], # is the same as Django UsernameValidator except for '@' symbol | ||
| help_text="30 characters or fewer. Can contain: letters, digits, underscores, dots, dashes, plus signs and spaces.", | ||
| error_messages={'invalid': "The username field must contain only letters, digits, underscores, dots, dashes," | ||
| "plus signs and spaces."}, |
There was a problem hiding this comment.
I guess this was added because of #1516. We no longer allow spaces in usernames though, so we should find a different solution for the specific use case that was listed in the issue. We might want to use a different validator when validating the profile form of a user that has a space in the username. I would revert this change and work on a better solution in a different branch and PR. Thanks!!!
| name = self.cleaned_data.get("name") | ||
| if BookmarkCategory.objects.filter(user=self.instance.user, name=name).exists(): | ||
| raise forms.ValidationError("This name already exists for a bookmark category of yours") | ||
| return name |
There was a problem hiding this comment.
I tried to change the name for a name of an already existing category and it did not complain, I guess this is work in progress?
| return HttpResponseRedirect(reverse("bookmarks-for-user", args=[request.user.username])) | ||
| else: | ||
| messages.add_message(request, messages.WARNING, "Form not valid. Check if provided category name already exists.") | ||
| return HttpResponseRedirect(reverse("bookmarks-for-user", args=[request.user.username])) |
There was a problem hiding this comment.
I see how you're trying to show the error message, but it should not work in this way. The change form is displayed in a modal, and if there are errors these should be shown in the modal as well. This is a bit complex, I need to remember how we used to do these things. Let me see if I remember as I continue reviewing the code.
There was a problem hiding this comment.
Ok I found the answer. The whole edit category in a modal thing should work in the same way that the sound flag form. Here is the view:
Line 922 in 836e974
This uses a "generic modal with form" util that displays a form in a modal, shows errors there if any. See other comments for more information.
| @@ -83,6 +83,31 @@ const bindConfirmationModalElements = (container) => { | |||
| }); | |||
| } | |||
|
|
|||
| // Edit modal logic | |||
| const bindEditModalElements = (container) => { | |||
There was a problem hiding this comment.
Instead of making a new type of modal, you should be using a "generic modal with form". You have an example with the modals used to flag a sound (see
). You should do a very similar implementation, with similar logic.
Issue(s)
#1755
#1577
Description
Changes in replies' definition of the recepient. Decide who will be the recepient according to who is requesting the reply message (request.user)
Addition of "edit_modal" to allow changes in category bookmark names, and a user interface to allow so. Bookmark category change allowed + checking that category name does not already exists.
Improvements to be made: show possible errors in modal dialog