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

Handle null text in first_paragraphs() #396

Merged

Conversation

DasSkelett
Copy link
Member

Problem

This link to a modpack on beta throws a 500: https://beta.spacedock.info/pack/1/some%20stuff

   File "/var/www/virtual/spacedock.info/htdocs/SpaceDock/KerbalStuff/../templates/layout.html", line 10, in top-level template code
     {%- block opengraph %}
   File "/var/www/virtual/spacedock.info/htdocs/SpaceDock/KerbalStuff/../templates/mod_list.html", line 9, in block "opengraph"
     <meta property="og:description" content="{{ mod_list.description | first_paragraphs | bleach }}">
   File "/var/www/virtual/spacedock.info/htdocs/SpaceDock/KerbalStuff/common.py", line 46, in first_paragraphs
     return '\n\n'.join(PARAGRAPH_PATTERN.split(text)[0:3])
 TypeError: expected string or bytes-like object

#385 made use of our first_paragraphs() function to get a short-ish text that can be put into tne og:description tag for mod packs.
However, the description column is nullable, and while it appears that new mod packs will always be initialized with an empty string, there can be old ones with NULL/None.

Pattern.split can only take strings as argument, and thus throws for None.

It would have been nice if mypy caught that, because first_paragraphs(test: str) explicitly asks for an argument that is not None, however as we already found out, it can't check Jinja templates. Additionally it seems to assume that ModList.description cannot be None.

Changes

first_paragraphs now accepts Optional[str] and returns an empty string if text is None.

This solution should be more future-proof than e.g. checking for None within the template before calling first_paragraph, as it catches all occurrences.

@DasSkelett DasSkelett added Type: Bug Area: Backend Related to the Python code that runs inside gunicorn Priority: Normal Status: Ready labels Oct 7, 2021
@HebaruSan HebaruSan changed the title Handle null text in first_paragraps() Handle null text in first_paragraphs() Oct 7, 2021
Copy link
Contributor

@HebaruSan HebaruSan left a comment

Choose a reason for hiding this comment

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

Yup, this is exactly what I had in mind for this fix. 👍

@DasSkelett
Copy link
Member Author

Whoops, now I see the typo on the commit message as well. Going to reword it later and merge afterwards

@DasSkelett DasSkelett force-pushed the fix/modpack-og-null-description branch from 5cb7791 to 0e8a0f6 Compare October 7, 2021 22:34
@DasSkelett DasSkelett merged commit 70b1583 into KSP-SpaceDock:alpha Oct 7, 2021
@DasSkelett DasSkelett deleted the fix/modpack-og-null-description branch October 7, 2021 22:35
This was referenced Oct 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Backend Related to the Python code that runs inside gunicorn Priority: Normal Status: Ready Type: Bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants