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

Bulletin Form for OCD PH #1470

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

Conversation

biplovbhandari
Copy link
Member

No description provided.

Name of the officer who is responsible to issue the bulletin
"""

return self.cap.get("bulletin_officer", "Name of Officer")
Copy link
Member

Choose a reason for hiding this comment

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

This stuff seems too dynamic to want to put into settings...although that is about to become more accessible via WebSetup.
Until then it would be better as a DB table? (Even with WebSetup, a DB table provides better UX)

Name of the method that is used to generate the bulletin for this deployment
"""

return self.cap.get("bulletin_method")
Copy link
Member

Choose a reason for hiding this comment

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

Seems odd to have this be configurable via settings like this?
What I would propose would be an S3Method for cap/alert/[id]/bulletin
This would still allow the template to define a custom function in config.py

if area_row:
# Show these buttons only if there is at least one area segment
cap_bulletin_method = settings.get_cap_bulletin_method()
if cap_bulletin_method:
Copy link
Member

Choose a reason for hiding this comment

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

If using the S3Method & DB configuration, the cap_bulletin table could be queried & if there is an undeleted row then proceed to add the buttons

@flavour
Copy link
Member

flavour commented Mar 19, 2018

Sorry for the delay in reviewing this - I just found a lot of GitHub mails in my spam folder!
Other than the suggestions above, the logo should really be in the static/themes/XX/img folder rather than main static/img

@biplovbhandari
Copy link
Member Author

Thanks for review @flavour . I will get back soon

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.

2 participants