Skip to content

Some stats page issues. #1134

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

Closed
wants to merge 1 commit into from
Closed

Some stats page issues. #1134

wants to merge 1 commit into from

Conversation

solarissmoke
Copy link
Contributor

  • If there is a connection problem when trying to create a site on the
    stats server, a fatal error occurs.
  • The stats tab shouldn't be visible if stats are disabled?

@rjmackay
Copy link
Contributor

rjmackay commented Jun 4, 2013

Theres some confusion here: the allow_stat_sharing setting includes some JS on the page to hit the Ushahidi stats tracker. These stats can later be view through that stats admin.

However the stats admin also shows other stats on categories etc. Which I don't think need the stats JS (correct me if you think I'm wrong.. haven't read that code recently). So we shouldn't hide the stats tab completely. Just the stats that aren't being collected.

The 2nd change to avoid the fatal error is good though. But can you drop the stats tab change?

@solarissmoke
Copy link
Contributor Author

@rjmackay my mistake. Have reverted the stats tab changes leaving only the error checking in create_site().

@rjmackay
Copy link
Contributor

rjmackay commented Jun 4, 2013

I've merge this in but reworded your commit message to describe what actually changed: eaf1a69

@rjmackay rjmackay closed this Jun 4, 2013
@solarissmoke
Copy link
Contributor Author

Thanks. I tried to be clever with reverting but it just overwrote my original commit.

@solarissmoke solarissmoke deleted the bug-stats-issues branch June 4, 2013 04:02
@rjmackay
Copy link
Contributor

rjmackay commented Jun 5, 2013

git rebase -i is great for cleaning up pull requests. Lets you squash commits together or reword the messages, etc.

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