Skip to content
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

Fix 500 during contact search; add comments and format files #4

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ms
Copy link

@ms ms commented Sep 10, 2022

Hello, I’m an early reviewer of the book.

A few minor fixes:

  • Searching throws a 500 because some of the mock data has null values. Added a check for None
  • Added a comment about the secret key so people don’t copy paste this line without thinking
  • Ran autopep8 to pretty-format the code on app and contact_model (hopefully that’s the right thing to do, I know even less Python than you say you do)

The first item leads me to ask: how does htmx handle HTTP errors? I didn’t see any indication of something being wrong in the UI (I had to open the devtools and then look at the terminal output).

I only tested searching. I’m happy to split this PR up if you prefer to keep commits more atomic in the repo.

Loving the book so far, great work!

@ms ms force-pushed the fix-500-during-search branch from 3d11356 to 1b51a0a Compare September 10, 2022 17:39
@@ -72,7 +74,7 @@ def all(cls, page=1):
def search(cls, text):
result = []
for c in cls.db.values():
if text in c.first or text in c.last or text in c.email or text in c.phone:
if (c.first != None and text in c.first) or (c.last != None and text in c.last) or (c.email != None and text in c.email) or (c.phone != None and text in c.phone):
Copy link
Author

Choose a reason for hiding this comment

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

This is the meat of the change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant