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

Changed SAMBRO homepage 500 error #1494

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

Conversation

Sleegreen
Copy link

As suggested by Nursix:
This happens due to missing map layer configurations - either because they have been deleted or not added to the database in the first place. This configuration error is logged, but no proper compensatory action is taken:
https://github.com/sahana/eden/blob/master/modules/templates/SAMBRO/controllers.py#L51

It would make sense to skip the entire map configuration (i.e. not call show_map at all) rather than just setting layer_id to None, and display a warning message to the user.

except:
current.log.error("Cannot find Layer for Map")
layer_id = None
Tkinter.messagebox.showinfo("Error", "Cannot find Layer for Map")
Copy link
Member

Choose a reason for hiding this comment

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

The message should be shown on the client-side, not on the server as normally there's no user there (hence the log message here). Besides, Tkinter is an optional module, and (for the same reason) not normally installed on the server.

The message can be shown either as HTML in place of the map (simply by putting it into the _map variable), or as response.error.

try:
layer_id = layer.layer_id
skipMap = False
Copy link
Member

Choose a reason for hiding this comment

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

Naming convention: not pascalCase, but all_lower_case with underscore as word separator

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