-
Notifications
You must be signed in to change notification settings - Fork 87
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
Enable a modal to edit bookmark categories #1799
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,7 +27,7 @@ | |
from django.shortcuts import get_object_or_404, render | ||
from django.urls import reverse | ||
|
||
from bookmarks.forms import BookmarkForm | ||
from bookmarks.forms import BookmarkForm, BookmarkCategoryForm | ||
from bookmarks.models import Bookmark, BookmarkCategory | ||
from sounds.models import Sound | ||
from utils.pagination import paginate | ||
|
@@ -85,6 +85,33 @@ def delete_bookmark_category(request, category_id): | |
return HttpResponseRedirect(reverse("bookmarks-for-user", args=[request.user.username])) | ||
|
||
|
||
@transaction.atomic() | ||
def edit_bookmark_category(request, category_id): | ||
|
||
if not request.GET.get('ajax'): | ||
return HttpResponseRedirect(reverse("bookmarks-for-user", args=[request.user.username])) | ||
|
||
category = get_object_or_404(BookmarkCategory, id=category_id, user=request.user) | ||
|
||
if request.method == "POST": | ||
edit_form = BookmarkCategoryForm(request.POST, instance=category) | ||
print(edit_form.is_bound) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 😲 |
||
if edit_form.is_valid(): | ||
category.name = edit_form.cleaned_data["name"] | ||
category.save() | ||
return JsonResponse({"success":True}) | ||
if not edit_form.is_valid(): | ||
print(edit_form.errors.as_json()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 😲 😲 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess you're referring to the second if block that just prints the form errors? I probably forgot to erase that since I think it was for debugging purposes. The first if statement I am quite sure it is required in order to update the category name in the database. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, the one for printing only |
||
else: | ||
initial = {"name":category.name} | ||
edit_form = BookmarkCategoryForm(initial=initial) | ||
|
||
tvars = {"category": category, | ||
"form": edit_form} | ||
return render(request, 'bookmarks/modal_edit_bookmark_category.html', tvars) | ||
|
||
|
||
|
||
@login_required | ||
@transaction.atomic() | ||
def add_bookmark(request, sound_id): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -85,19 +85,22 @@ const bindConfirmationModalElements = (container) => { | |
|
||
// Logic to bind default modals | ||
|
||
const handleDefaultModal = (modalUrl, modalActivationParam, atPage) => { | ||
if ((atPage !== undefined) && modalUrl.indexOf('&page') == -1){ | ||
modalUrl += '&page=' + atPage; | ||
} | ||
const handleDefaultModal = (modalUrl, modalActivationParam) => { | ||
handleGenericModal(modalUrl, undefined, undefined, true, true, modalActivationParam); | ||
} | ||
|
||
const handleDefaultModalWithForm = (modalUrl, modalActivationParam) => { | ||
handleGenericModalWithForm(modalUrl, undefined, undefined, (req) => {showToast('Form submitted succesfully!')}, undefined, true, true, modalActivationParam, true); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are enabling the reload on success by default, and also not providing a way to disable it unless There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure that I fully understood your suggestion because to my understanding you do not need to check anything in the bind function if the parameter is sent properly in handeDefaultModalWithForm. By now my proposal would be to:
There are other implemented changes you proposed in another branch (as in messageOnSuccess). Anyway, the result seems to work fine and would allow not to reload the page by default (only when specified in the element). Did I understand it correctly? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, it might be a good idea to simply pass |
||
} | ||
|
||
const bindDefaultModals = (container) => { | ||
bindModalActivationElements('[data-toggle="modal-default"]', handleDefaultModal, container); | ||
bindModalActivationElements('[data-toggle="modal-default-with-form"]', handleDefaultModalWithForm, container); | ||
} | ||
|
||
const activateDefaultModalsIfParameters = () => { | ||
activateModalsIfParameters('[data-toggle="modal-default"]', handleDefaultModal); | ||
activateModalsIfParameters('[data-toggle="modal-default-with-form"]', handleDefaultModalWithForm); | ||
} | ||
|
||
|
||
|
@@ -195,7 +198,7 @@ const handleGenericModal = (fetchContentUrl, onLoadedCallback, onClosedCallback, | |
}; | ||
|
||
|
||
const handleGenericModalWithForm = (fetchContentUrl, onLoadedCallback, onClosedCallback, onFormSubmissionSucceeded, onFormSubmissionError, doRequestAsync, showLoadingToast, modalActivationParam) => { | ||
const handleGenericModalWithForm = (fetchContentUrl, onLoadedCallback, onClosedCallback, onFormSubmissionSucceeded, onFormSubmissionError, doRequestAsync, showLoadingToast, modalActivationParam, dataReloadOnSuccess) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
// This version of the generic modal is useful for modal contents that contain forms which, upon submission, will return HTML content if there were form errors | ||
// which should be used to replace the current contents of the form, and will return a JSON response if the form validated correctly in the backend. That JSON | ||
// response could include some relevant data or no data at all, but is used to differentiate from the HTML response | ||
|
@@ -218,6 +221,9 @@ const handleGenericModalWithForm = (fetchContentUrl, onLoadedCallback, onClosedC | |
if (onFormSubmissionSucceeded !== undefined){ | ||
onFormSubmissionSucceeded(req); | ||
} | ||
if(dataReloadOnSuccess == true){ | ||
location.reload() | ||
} | ||
} else { | ||
// If the response is not JSON, that means the response are the HTML elements of the | ||
// form (including error warnings) and we should replace current modal HTML with this one | ||
|
@@ -278,4 +284,4 @@ const handleGenericModalWithForm = (fetchContentUrl, onLoadedCallback, onClosedC | |
}, onClosedCallback, doRequestAsync, showLoadingToast, modalActivationParam) | ||
} | ||
|
||
export {activateModal, dismissModal, handleGenericModal, handleGenericModalWithForm, handleDefaultModal, bindModalActivationElements, bindConfirmationModalElements, activateModalsIfParameters, bindDefaultModals, activateDefaultModalsIfParameters}; | ||
export {activateModal, dismissModal, handleGenericModal, handleGenericModalWithForm, handleDefaultModal, handleDefaultModalWithForm, bindModalActivationElements, bindConfirmationModalElements, activateModalsIfParameters, bindDefaultModals, activateDefaultModalsIfParameters}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
{% extends "molecules/modal_base.html" %} | ||
{% load util %} | ||
|
||
{% block id %}editBookMarkCategory{% endblock %} | ||
{% block extra-class %}{% endblock %} | ||
{% block aria-label %}Edit Bookmark Category{% endblock %} | ||
|
||
{% block body %} | ||
<div class="col-12"> | ||
<div class="text-center"> | ||
<h4 class="v-spacing-5">Edit bookmark Category</h4> | ||
<form method="post" action="{% url 'edit-bookmark-category' category.id %}?ajax=1." class = "disable-on-submit">{% csrf_token %} | ||
{{form}} | ||
<button class="btn-primary v-spacing-top-3">Change bookmark category name</button> | ||
</form> | ||
</div> | ||
<div class="v-spacing-4"> | ||
|
||
</div> | ||
</div> | ||
{% endblock %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This error message could be improved. Maybe "You have already created a bookmark category with that name"