Newsletter Updates #206
Newsletter Updates #206
Conversation
| 'start_date': forms.DateInput(attrs={'type': 'date', 'class': 'border p-2 w-full'}), | ||
| 'end_date': forms.DateInput(attrs={'type': 'date', 'class': 'border p-2 w-full'}), | ||
| 'timing': forms.TimeInput(attrs={'type': 'time', 'class': 'border p-2 w-full'}), |
There was a problem hiding this comment.
These classes are not handled by the base form?
| timing = models.TimeField(null=True, blank=True) | ||
| link = models.URLField(null=True, blank=True) | ||
| archived = models.BooleanField(default=False) | ||
| created_at = models.DateTimeField(auto_now_add=True) |
There was a problem hiding this comment.
If you have created at, also add updated at
| path("manage_announcements/", views.manage_announcements, name="newsletter_manage_announcements"), | ||
| path("new_announcement/", views.new_announcement, name="newsletter_new_announcement"), | ||
| path("edit_announcement/<int:pk>", views.edit_announcement, name="newsletter_edit_announcement"), | ||
| path("toggle_announcement/<int:pk>", views.toggle_announcement, name="newsletter_toggle_announcement"), | ||
| path("delete_announcement/<int:pk>", views.delete_announcement, name="newsletter_delete_announcement") |
There was a problem hiding this comment.
Use resource API style
/announcements/new, /announcements/manage, /announcement/edit, etc.
| {% load static %} | ||
|
|
||
| {% block title %} | ||
| Newsletter | New |
There was a problem hiding this comment.
New for update announcement?
Also use consistent naming so that devs are not confused in the future xD Use Edit announcement everywhere (similar to the file name)
| def toggle_announcement(request, pk): | ||
| announcement = get_object_or_404(Announcement, pk=pk) | ||
| announcement.archived = not announcement.archived | ||
| announcement.save() | ||
| string = "archived" if announcement.archived else "unarchived" | ||
| messages.success(request, f"Announcement {string} successfully") | ||
| return redirect('newsletter_manage_announcements') |
There was a problem hiding this comment.
Shouldn't be like this. Use the edit announcement view itself because
- We already have a view that updates the model.
- Any APIs that can update the DB should always be POST/PATCH requests. GET requests should never be able to update the DB.
| def delete_announcement(request, pk): | ||
| announcement = get_object_or_404(Announcement, pk=pk) | ||
| announcement.delete() | ||
| messages.success(request, "Announcement deleted successfully") | ||
| return redirect('newsletter_manage_announcements') |
There was a problem hiding this comment.
I have a few opinions on this, can discuss further - I don't think we should delete data. Let it be there, on deletion, just archive it but not actually delete it.
Again, cannot be a GET request, has to be a POST/PATCH/DELETE request.
| @@ -12,7 +12,7 @@ class CorpusUserAdmin(UserAdmin): | |||
| (None, {"fields": ("email", "password")}), | |||
| ( | |||
| "Personal Info", | |||
| {"fields": ("first_name", "last_name", "phone_no", "gender")}, | |||
| {"fields": ("first_name", "last_name", "phone_no", "gender", "profile_pic")}, | |||
There was a problem hiding this comment.
Ideally shouldn't be part of this diff since it is something separate and not pertaining to the diff.....
But I'll allow it once xD
Description
Features being implemented -
profile_picattribute of UsersType of Change
What types of changes does your code introduce?
Put an
xin the boxes that apply