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

Add agency name to report #198

Merged
merged 1 commit into from
Mar 2, 2017
Merged

Add agency name to report #198

merged 1 commit into from
Mar 2, 2017

Conversation

jmhooper
Copy link
Member

This command adds an option for specifying an agency in the CLI options. This allows the name of the agency the data is being pulled for to be included in the final report, and in the data that is written to the database.

Ref #194

Hold for #197

@@ -1,3 +1,4 @@
# Agency for International Development
export ANALYTICS_REPORT_IDS="ga:68380943"
export AGENCY_NAME=agency-international-development
export AWS_BUCKET_PATH=data/agency-international-development
Copy link
Contributor

Choose a reason for hiding this comment

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

Why can't the analytics script itself read the AGENCY_NAME env var, instead of having it passed in on a new command line flag?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea

Copy link
Member Author

Choose a reason for hiding this comment

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

Just moved things around so the name is pulled from the AGENCY_NAME env var.

index.js Outdated
@@ -64,6 +64,8 @@ var run = function(options) {

if (options.debug) console.log("[" + report.name + "] Saving report data...");

if (options.agency) data.agnecy = options.agency;
Copy link
Member Author

Choose a reason for hiding this comment

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

Marking a typo here

@jmhooper jmhooper force-pushed the jmhooper-agency-name branch 2 times, most recently from bc5fc9f to b9c8f9a Compare March 1, 2017 22:37
@jmhooper
Copy link
Member Author

jmhooper commented Mar 1, 2017

I've moved the changes that were causing this to be held into #197. So now #197 is holding for this, and this is no longer holding for #197.

@jmhooper jmhooper changed the title [HOLD] Add --agency option to the CLI Add agency name to report Mar 1, 2017
README.md Outdated
@@ -232,6 +232,12 @@ analytics --frequency=realtime
analytics --publish --debug
```

* `--agency` - provide an agency name to associate with the report
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the documentation also need to be updated to not use the --agency flag?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, my bad

Copy link
Member Author

Choose a reason for hiding this comment

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

Amended the commit

This commit adds the agency name to the reports the analytics reporter produces. When #197 is merged, this will be used to write associate agencies with reports in our database.

The agency is specified by setting the `AGENCY_NAME` environment variable which is picked up by the config.

Ref #194
@konklone konklone merged commit 85d515a into master Mar 2, 2017
@konklone konklone deleted the jmhooper-agency-name branch March 2, 2017 15:55
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.

None yet

2 participants