Skip to content
This repository has been archived by the owner on Feb 24, 2024. It is now read-only.

Basic Email Templating, Confirmation Logic & Mobile Styling Fixes #118

Merged

Conversation

JackRichard889
Copy link
Collaborator

Can close #84, #99, and #117.

I made a text and HTML version of the "email templates," which is really just one mediocre template right now. Django recommends an alternate version of the email for weird browsers and email clients.

Some of the parts of the app looked really strange on mobile, so I tried to fix it as much as possible on smaller screen sizes.

The confirmation logic includes a URL to confirm a reservation and two Django template filters to calculate confirmed reservations from a set. Also fixed all of the "available" and "taken" counts that had to be changed.

@JackRichard889 JackRichard889 self-assigned this Mar 17, 2022
This was linked to issues Mar 17, 2022
Copy link
Contributor

@seamussmith seamussmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I probably should have been a bit more clear about where to put the email templates.

When I said I wanted the email templates in the /email/layouts/ directory, I was referring to the template import path name, the path you use to reference to the template. I probably should have made this more clear, and I plan to update the documentation on our template directory schema.

I'll give you a rundown on how we store our templates:

Django looks at the templates folder of each app in a project and allows developers to refer to them without using a relative directory, like for instance, the template with the path /app/templates/app/pages/index.html can be referred to as /app/pages/index.html. The reason why the front-end templates are in an extra /app/ folder is to create a separation of concerns. If we have all of our templates in the base template folder, we would run into name collisions and it would be, in general, very messy.

When we need templates for other things, like emails, we would have another folder called email in our /app/templates/ folder. The folder structure could probably follow in the footsteps of our app templates, with the whole components layouts pages schema. pages in this case could be renamed to something more sensible like emails.

Our templates usually also extend from a base template to reduce code duplication on things like headers and footers. In the case of emails, our base template would most likely have a header and a signature at the end. This template would sensibly go in the /emails/layouts/base.html template directory.

I hope this clears any confusion.

@@ -14,6 +14,7 @@
path('schedule/<schedule:schedule>/create-timeslots', views.create_timeslots, name='create-timeslots'),
path('schedule/<schedule:schedule>/<datetime:date>/reserve/<timeslot:timeslot>', views.reserve_timeslot, name='reserve-timeslot'),
path('schedule/<schedule:schedule>/<datetime:date>/view/<timeslot:timeslot>', views.view_reservations, name='view-reservations'),
path('confirm/<int:reservation>', views.confirm_reservation, name='confirm-reservation'),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should probably use the new model URL converters. We have them for Schedule, TimeSlot, and Reservation. These URL converters allow us to eliminate that one line of code used for fetching the data from the database. You can look at how these converters are implemented in the app.converters module.

In this case here, you would change the <int:reservation> to <reservation:reservation> to take advantage of the URL converter.

@ghost
Copy link

ghost commented Mar 18, 2022 via email

@JackRichard889
Copy link
Collaborator Author

Fixed the issues with the email templates and the URL for confirmation. Also added cancellation URL to close #120. Didn't have time today to convert primary keys to UUIDs for reservations. There were some issues with using custom converters with the UUID data type... Good luck

@JackRichard889 JackRichard889 linked an issue Mar 18, 2022 that may be closed by this pull request
Copy link
Contributor

@seamussmith seamussmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I can implement the new converter logic myself.

@seamussmith seamussmith merged commit 47172f9 into worcestertechnicalhighschool:master Mar 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants