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.

Sign up for GitHub

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

Jump to bottom

adding messages to home pages #76

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

adding messages to home pages #76

wants to merge 2 commits into from

Conversation

moshe742
Copy link
Member

@moshe742 moshe742 commented Jun 3, 2018

  • adding a view with the context data that inherits from TemplateView
  • adding in the template all that is needed
  • changed the urls file to get the homepage class i created on views
  • added in the admin the 2 pages (on the server, can delete if not approved)

@moshe742
adding messages to home pages
a29dc3b 
* adding a view with the context data that inherits from TemplateView
* adding in the template all that is needed
* changed the urls file to get the homepage class i created on views
* added in the admin the 2 pages (on the server, can delete if not approved)
@shaib
Copy link
Member

shaib commented Jun 3, 2018

This has two problems:

  • if there are no pages titled "messages" and "messages_hebrew", the view will raise a DoesNotExist exception and the home-page will show a server error
  • If someone puts ---- in the message text, e.g. to separate messages, then the messages will miraculously disappear

I suggest, instead, that the view will look for the pages, and if they aren't there, that will be the mark that there are no messages. The messages can still be always present in the database, just with a changed title.

@moshe742
making better work
95d22ac 
* using filter on view instead of get
* checking if there is data and if there is creating the context
* checking if the context has the data and if so showing the relevant html
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MeirKriheli MeirKriheli Awaiting requested review from MeirKriheli

@shaib shaib Awaiting requested review from shaib

@AmitAronovitch AmitAronovitch Awaiting requested review from AmitAronovitch

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@moshe742 @shaib