elthran / BookingSystem

A working booking/calendar system for clients to take orders and create a schedule
0 stars 0 forks source link

Need help cleaning up some code #24

Open elthran opened 6 years ago

elthran commented 6 years ago

templates/edit_store_hours.html

<form method="POST" action="." accept-charset="UTF-8" role="form">
  <!-- LoginForm -->
  {{ form.csrf_token }}
  Closed Monday: {{ render_field(form.monday_closed) }}
  {% if monday_closed %}
    Monday from {{ render_field(form.monday_start, autofocus="", disabled=True) }} to {{ render_field(form.monday_end, disabled=True) }}
  {% else %}
    Monday from {{ render_field(form.monday_start, autofocus="") }} to {{ render_field(form.monday_end) }}
  {% endif %}
  Closed Tuesday: {{ render_field(form.tuesday_closed) }}
  {% if tuesday_closed %}
    Monday from {{ render_field(form.tuesday_start, autofocus="", disabled=True) }} to {{ render_field(form.tuesday_end, disabled=True) }}
  {% else %}
    Monday from {{ render_field(form.tuesday_start, autofocus="") }} to {{ render_field(form.tuesday_end) }}
  {% endif %}
<br><button type="submit" name="submit">Submit</button>
</form>

Any way to improve this?

Even worse: routes/business/edit_store_hours.py

location = Location.query.get(current_user.location_id)
    if location.hours:
        for hour in location.hours:
            if hour.day == 1:
                monday_closed = hour.closed
                monday_start = hour.start
                monday_end = hour.end
            elif hour.day == 2:
                tuesday_closed = hour.closed
                tuesday_start = hour.start
                tuesday_end = hour.end
    else:
        monday_start = 1
        monday_end = 12
        monday_closed = False
        tuesday_start = 1
        tuesday_end = 12
        tuesday_closed = False
    form = AvailabilityForm(request.form,
                            monday_closed=monday_closed, monday_start=monday_start, monday_end=monday_end,
                            tuesday_closed=tuesday_closed, tuesday_start=tuesday_start, tuesday_end=tuesday_end)
    form.monday_start.choices = [(i, str(i)+":00") for i in range(1,13)]
    form.monday_end.choices = [(i, str(i)+":00") for i in range(1,13)]
    form.tuesday_start.choices = [(i, str(i) + ":00") for i in range(1, 13)]
    form.tuesday_end.choices = [(i, str(i) + ":00") for i in range(1, 13)]
    if form.validate_on_submit():
        monday_closed = form.monday_closed.data
        monday_length = form.monday_end.data - form.monday_start.data
        tuesday_closed = form.tuesday_closed.data
        tuesday_length = form.tuesday_end.data - form.tuesday_start.data
        if monday_length <= 0 or tuesday_length <= 0:
            flash("Can't add a negative time", "error")
            return redirect(url_for('edit_store_hours'))
        hours = Hour.query.filter_by(location_id=location.id).all()
        for hour in hours:
            db.session.delete(hour)
        print(location.hours)
        hour = Hour(location.id, 1, form.monday_start.data, monday_length, monday_closed)
        db.session.add(hour)
        db.session.commit()
        hour = Hour(location.id, 2, form.tuesday_start.data, tuesday_length, tuesday_closed)
        db.session.add(hour)
        db.session.commit()

Everything about this is awful. Please take a look at these two and help make it more generic before I add the other days of the week. Maybe making day objects that have a opening time and closing time etc, and iterating through a bunch? What I have is awful.

elthran commented 6 years ago

A store will have a set of hours. One for each day of the week. Anytime you make a change it should delete all of the hours for your business and rebuild it.

coding

Here is the one from the site Melissa uses. You can see, to make a change it brings up all your hours. You adjust them all as you like and submit. Mine is the same. You bring up all the hours at once and submit, so it erases any database content involving your store location and replaces it with the updated version. There will be one entry for each day of the week and then a holidays entry. so 8 entries per business. (each Location will have 8 Hours entries in the Hours table). Each entry has a

That's all I currently need. It all works. I don't need help making it work. Just improving the code because it's shit and hard to read. I just wrote the explanation so you can understand it more easily!

elthran commented 6 years ago

Also the very first if/else statement will be removed. It was in place because there was no hours object when you first loaded it and it needed to check for one to find the preset values. But I will add a prebuilt hours object for eachlocation (defaulting to being closed every day and with hours of 9-5, so when you open i, it starts at nice hours). So dont worry about improving that very first if/else, but everything else needs help being more generic

klondikemarlen commented 6 years ago

I have an idea ... I posted a bunch of stuff in comments on your commits. Would you be able you review (an merge) my current pull request first though? If you wait too long it won't be able to merge cleanly and I'll have to redo it.

klondikemarlen commented 6 years ago

In brief it looks like this:

location.availablities -> ordered list of Availabilities (sorted by day_of_the_week, start_time)

Avaiability -closed -start_time -duration -day_of_the_week (as an int) -> use CONSTANTS for readability i.e SUNDAY = 0, MONDAY = 1

Display code (in jinja but I was too lazy to write it that way): for availability in location.availabilities: # ordered list render_field(availability) -> This should be a custom widget with Open or Closed states.

Output Sun: Unavailable # closed state ->add time slot # additional slot on Sunday Mon: _____ to __ x # open state -> add time slot # additional slot on Monday etc.

https://wtforms.readthedocs.io/en/latest/widgets.html#custom-widgets, check out my first pull request https://github.com/elthran/BookingSystem/pull/22 for an example