-
Notifications
You must be signed in to change notification settings - Fork 506
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
1595 intercom #1718
1595 intercom #1718
Conversation
@@ -0,0 +1,192 @@ | |||
@usersFixture @oauth2Skip @users |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need to move this file to fit the new layout
Changes Unknown when pulling 08e5c2b on 1595-intercom into ** on develop**. |
Notes:
|
Looks good in intercom! passes QA. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So my main concern here is that the events are tightly coupled with the data we want to give intercom. I think ideally they would be simpler and multiple per repo: ie. a user update event, that would include all the user data, or a form update event that would the entire form.
Any further info gathering should then be trigger from the listener ie. if we want to grab the form count we should callback to the repo from the listener. That way we can make the updates async later on.
In the case of users I think ideally we should try to push all deployment admins directly into intercom. Probably not members since they're not really our users, rather our deployers users.
@@ -73,6 +73,11 @@ public static function init() | |||
and Kohana::$config->load('features.private.enabled'); | |||
}); | |||
|
|||
// Intercom config settings | |||
$di->set('site.intercomAppToken', function() use ($di) { | |||
return Kohana::$config->load('site.intercomAppToken'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Grouping this into site
kind of implies that it can change per deployment... it doesn't.
Site is also user editable... this shouldn't be right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmmmm, I hadn't thought of that. I added it to site to allow users to edit it for their deployment not with the intention that they can push updates to it. Would it be better under something like third_party_app_config?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea. Its not super clear where it should live. Probably just thirdparty
or something similar would work. I think you could just call getenv()
directly here and skip the kohana config entirely
@@ -672,6 +677,22 @@ public static function init() | |||
$di->setter['Ushahidi_Listener_PostListener']['setWebhookRepo'] = | |||
$di->lazyGet('repository.webhook'); | |||
|
|||
// Add Intercom Listener to Config |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be too late for this.. but could we move these into src/App ? (fine if not, I'll move them in the lumen branch instead)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If i'm going to make the other changes I can move this as well.
|
||
// New User - set their deployment created date | ||
if ($key === 'first_login') { | ||
$intercom_data['deployment_created_date'] = date("Y-m-d H:i:s"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems likely to be error prone. Could we do something like inject a real 'deployment_created_date' via migrations? ... or we push deployment creation date from the platform-cloud-interface
codebase instead since it actually knows when deployments are created.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In retrospect, I really dislike this the most. I think what we could do is insert the original user into intercom when the deployment is created in cloud interface then update the user when they complete first_login separately. I don't think that is hugely difficult. The app token can be push into cloud interface from the same ansible.
@@ -49,21 +55,49 @@ public function update(Entity $entity) | |||
|
|||
$config = \Kohana::$config->load($group); | |||
|
|||
// Intercom count datasources | |||
if ($group === 'data-provider') { | |||
$intercomData['num_data_sources'] = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems very closely coupled for something that is supposedly loosely coupled...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I'm not sure where else we could intercept the datasources. The ideal would really be to just have an occasional query that gives us stats on deployments. That would work best in the context of the single DB though. Then if that's needed in intercom we could push from our own stats collector.
@@ -77,6 +83,15 @@ public function create(Entity $entity) | |||
// UpdateRepository | |||
public function update(Entity $entity) | |||
{ | |||
|
|||
// If orignal Form update Intercom if Name changed | |||
if ($entity->id === 1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whats with the magic number?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The original form id, not sure how else to identify it.
@@ -133,6 +138,9 @@ public function isUniqueEmail($email) | |||
// RegisterRepository | |||
public function register(Entity $entity) | |||
{ | |||
|
|||
$this->updateIntercomUserCount(1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it'd make more sense to push the user info itself into Intercom when a new user is added, particularly admins. Then we can actually message all admins from a deployment via intercom.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would definitely agree with that. The issue asked for user count though but it would be vastly easier to push the user itself and then intercom filter by deployment to get the count.
Hrm. I somehow missed that this had already merged :( |
It can be improved still :) |
So for the user event I would agree that pushing the entire admin object might be better. For the form, my understanding of the required data was just whether or not the Primary Form had changed name. Part of the issue with the required data is that it is individual piece rather than whole objects. In retrospect, it would have been better to take the required data and abstract it out to some kind of object that can be passed. There is an issue with pushing each user, which one is the primary? If I push 5 admins but I only push an update that the primary form name has been changed, it's only going to go intercom user that changed it so all the other admins won't have that data and it will be confusing if it's done by a non-admin so there will be a new user created. Intercom has Company objects though and it seems better to use that and then users can be associated with companies. Do you think that'd work? |
That should be a property of the company not the user. |
* 1595 intercom (#1718) * adding intercom listener to capture user events * Adding additional intercom events * Adding first login * Fixing bracket * Adding intercom lib * Updating listener * Add laravel/homestead box and remove puppet scripts (#1722) * Updating composer * Updating composer.lock from fresh file * Fixing tests * Update users.feature * Remove redundant file * Fixed var name * Fix default for existing tasks * adding new migration to show_when_published (#1731) * Changing column default for task visibility * Repair linting * Fix Waffle badge Extra space was breaking markdown. * Adding total to geojson collection (#1739) * Adding total to geojson collection * Adding explanatory note about geojson addition * Adjusting unmapped request to incorporate it into stats (#1738) * Adjusting unmapped request to incorporate it into stats and changing the way in which unmapped is caluclated * Fixing failing test * Fixing get total count * Geojson chunking (#1744) * Adding total to geojson collection * Adding explanatory note about geojson addition * Removing unneeded logging * Fixed issue when no posts returned by search * Update linting setup and fix any new lint errors (#1742) * Update phpcs * Add ruleset for src/ * Fix linting on src/ * Lint tests * Make tests PSR4 compliant * Split Kohana bootstrap out of Feature Context so we can ignore it for CS * Fix spec ruleset * Fix linting on migrations * Fixing missing db config for unmapped query * Enable clustering by default * Only add `tags` type attributes to the "post fields" not tasks. * Remove category fields from tasks * Turn posts_tags into a full post value table, and avoid duplicating info - Add extra field to posts_tags - Remove tags from post_varchar - Add Post/Tags validator - Add Post/Tags repo * Don't allow making tags attributes private * Handle legacy tags values and save into first tags attribute. This isn't 100% compatible, but we at least save tags *if* the form has a tags attribute * Make forms tests work in isolation * Stop limiting tags by form, just limit tags per attribute - Stop using forms_tags table - Just use form_attribute.options instead * Rename tags to categories in error messages * Format form tags and tag children as relations, remove tag forms formatter * Remove tags?formId filter from Tag repo We've removed the relevant table, so lets remove the code too! * Fix validating tags when saving a post Typo when injecting tags_repo was breaking it * Fix validating tags when saving a post Typo when injecting tags_repo was breaking it * Add TagRepository interface to use in Post/Tags Validator - Use tag field to set tags in test - Add TagRepository Interface - Implement interface in Tag Repo * Ignore form.tags if passed when saving Form entities Ignore computed attributes on form * Add test for removing tag from attribute options when deleted * Test that form.tags value is ignored during form update * Make child tags top level when parent is deleted, don't delete them Update foreign key to ON DELETE SET NULL, not CASCADE * Reset fixtures before testing removing a tag Fix to c55dbfe * Parse tag names when set as post value (not just post.tags) Parse tags as name or id when sent in post.values. We already do this with post.tags * Update to use checklist for testing * Fixing error message for tag (#1760) * Fixing error message for tag * adding name to error-message when importing posts with categories * removing children from tag-object before updating * removing tags from form-object before saving form * Incorrect length requirement
* Fixing markdown field size bug * Set up Ushahidi\App namespace and move the simple classes Migrating the easy, non kohana related stuff into a namespace * Format for PSR2 * Disable slack notifications from travis * Adding survey editor update (#1662) * Adding survey editor update * Field locking fully working, next fix broken tests * Fixing tests * Update Vagrantfile * Remove extra files * Updating counts * Adding entity fields * Removing unneeded include, simplifying function * Adding task restriction * Adding form stage and attribute restriction * Fixed stage tests * Updated attribute filitering and added tests * Fixed linting * Fixing base data set * Fixing tests * removing extraneous char * Docs: Add more details on use case internals * 1511 field level locking (#1690) * Adding survey editor update * Field locking fully working, next fix broken tests * Fixing tests * Update Vagrantfile * Remove extra files * Updating counts * Adding entity fields * Removing unneeded include, simplifying function * Adding task restriction * Adding form stage and attribute restriction * Fixed stage tests * Updated attribute filitering and added tests * Fixed linting * Fixing base data set * Fixing tests * removing extraneous char * Fixing access permissions * Fixing test * Improve categories (#1670) * adding forms-tags-table #1397 * adding helper-functions for updating forms-tags-relations #1397 * updating forms_tags-table when updating a tag #1397 * updating forms_tags-table when updating/creating an attribute #1397 * adding search-fields for tags #1397 * adding tags to form #1397 * updating forms_tags-table when adding a new survey #1397 * changing naming to tags #1397 * removing updating form_tags when saving an attribute, had got the relation wrong #1397 * lint * not returning all posts when search->tags is empty #1397 * updating form_attributes when tag is deleted #1397 * updating forms-tags relation when form is updated #1397 * removing old forgotten debug-logging * resetting post-category-filtering to normal #1397 * adding child-tag-ids to tag #1397 * adding migrations for tags #1397 * fixing failing test #1397 * linting + fix comment from review #1397 * adding migration to move post-tag-values to post_varchar #1397 * Adds language to user (#1659) * adding language to user #1651 * fixing linting and removing comment #1651 * lint #1651 * Fix from QA (#1705) * adding forms-tags-table #1397 * adding helper-functions for updating forms-tags-relations #1397 * updating forms_tags-table when updating a tag #1397 * updating forms_tags-table when updating/creating an attribute #1397 * adding search-fields for tags #1397 * adding tags to form #1397 * updating forms_tags-table when adding a new survey #1397 * changing naming to tags #1397 * removing updating form_tags when saving an attribute, had got the relation wrong #1397 * lint * not returning all posts when search->tags is empty #1397 * updating form_attributes when tag is deleted #1397 * updating forms-tags relation when form is updated #1397 * removing old forgotten debug-logging * resetting post-category-filtering to normal #1397 * adding child-tag-ids to tag #1397 * adding migrations for tags #1397 * fixing failing test #1397 * linting + fix comment from review #1397 * adding migration to move post-tag-values to post_varchar #1397 * fixing id-error when updating a survey #1397 * Release v3.6.4 (#1707) **New Features** - Added Field Level Locking to Surveys - Improved Survey UX - Added Task Level Visibility - Added Webhook Support - Added Webhook UX - Added Webhook events fro Create, Update of Post - Added Survey Duplication - Added Task Duplication - Added Support for OSS deployment with Ushahidi Mobile - Added Deeplinking for iOS and Android - Added Banner ads for iOS and Android - Adding Location Search Picker for Maps in Posts - Added reply to twitter via Post feature - Added support for hierarchical categories - Added Category field type to Survey - Added Markdown field type to Survey - Add tags dynamically directly from Posts - Added language switching for all users **Minor improvements and fixes** - Adding uglify-friendly code to colors - Confirming on delete for unsaved surveys, tasks and fields - Upgrade node to v6 - Resolved twitter duplication bug - Display number of unmapped posts to timeline - Moved Savedsearches to Modal - Moved Filter to Modal * Update composer dependencies (#1666) * Rearranging tests out of application/ into tests/ * Update composer packages to recent versions - Update PHPUnit - Update Behat - Update PHPSpec - Update aura/di to remove custom fork - Switch flysystem to 1.0 not 1.1 dev - Update phinx - Update phpunit tests for new version - Update behat tests for new version * Fix DI to work without auto resolving enabled * Run codeship builds on PHP 5.6 * Fix travis builds * Remove bin/tests * Fix phpunit tests failing while trying to backup globals Tests were failing with "Serialization of 'Closure' is not allowed" * Ignore tests/ for linting * fixup lint * Fix PHPUnit code coverage, and fix some bugs too * Don't inject Signer into webhook command Signer is just overwritten anyway, no need to inject it. Injection was broken because we didn't pass an auth token * Fix bin/ushahidi for new symfony/console version * Fixup coverage reporting from travis * Try to fix sending files to coveralls * Add PHP 7.1 to travis builds * Exclude php 7.1+coverage build * Add laravel/homestead box and remove puppet scripts * Update ohanzee/database to our fork, and select only aggregates when counting Mysql 5.7 doesn't like selecting both grouped and non grouped info in the same query. So we're not resetting the select fields before adding aggregates (ie. count) to the query * Remove posts?current_stage filter as its not working, and fails hard in mysql 5.7 * Set mysql strict mode in travis * Reset order by clause when fetching count queries too * Fix posts stats to work with mysql only_full_group_by mode * Output mysql version from travis builds * Use MAX() instead of ANY_VALUE() in queries ANY_VALUE only exists in mysql 5.7+ * Merging Develop into Master (#1730) * Fixing markdown field size bug * Set up Ushahidi\App namespace and move the simple classes Migrating the easy, non kohana related stuff into a namespace * Format for PSR2 * Disable slack notifications from travis * Adding survey editor update (#1662) * Adding survey editor update * Field locking fully working, next fix broken tests * Fixing tests * Update Vagrantfile * Remove extra files * Updating counts * Adding entity fields * Removing unneeded include, simplifying function * Adding task restriction * Adding form stage and attribute restriction * Fixed stage tests * Updated attribute filitering and added tests * Fixed linting * Fixing base data set * Fixing tests * removing extraneous char * Docs: Add more details on use case internals * 1511 field level locking (#1690) * Adding survey editor update * Field locking fully working, next fix broken tests * Fixing tests * Update Vagrantfile * Remove extra files * Updating counts * Adding entity fields * Removing unneeded include, simplifying function * Adding task restriction * Adding form stage and attribute restriction * Fixed stage tests * Updated attribute filitering and added tests * Fixed linting * Fixing base data set * Fixing tests * removing extraneous char * Fixing access permissions * Fixing test * Improve categories (#1670) * adding forms-tags-table #1397 * adding helper-functions for updating forms-tags-relations #1397 * updating forms_tags-table when updating a tag #1397 * updating forms_tags-table when updating/creating an attribute #1397 * adding search-fields for tags #1397 * adding tags to form #1397 * updating forms_tags-table when adding a new survey #1397 * changing naming to tags #1397 * removing updating form_tags when saving an attribute, had got the relation wrong #1397 * lint * not returning all posts when search->tags is empty #1397 * updating form_attributes when tag is deleted #1397 * updating forms-tags relation when form is updated #1397 * removing old forgotten debug-logging * resetting post-category-filtering to normal #1397 * adding child-tag-ids to tag #1397 * adding migrations for tags #1397 * fixing failing test #1397 * linting + fix comment from review #1397 * adding migration to move post-tag-values to post_varchar #1397 * Adds language to user (#1659) * adding language to user #1651 * fixing linting and removing comment #1651 * lint #1651 * Fix from QA (#1705) * adding forms-tags-table #1397 * adding helper-functions for updating forms-tags-relations #1397 * updating forms_tags-table when updating a tag #1397 * updating forms_tags-table when updating/creating an attribute #1397 * adding search-fields for tags #1397 * adding tags to form #1397 * updating forms_tags-table when adding a new survey #1397 * changing naming to tags #1397 * removing updating form_tags when saving an attribute, had got the relation wrong #1397 * lint * not returning all posts when search->tags is empty #1397 * updating form_attributes when tag is deleted #1397 * updating forms-tags relation when form is updated #1397 * removing old forgotten debug-logging * resetting post-category-filtering to normal #1397 * adding child-tag-ids to tag #1397 * adding migrations for tags #1397 * fixing failing test #1397 * linting + fix comment from review #1397 * adding migration to move post-tag-values to post_varchar #1397 * fixing id-error when updating a survey #1397 * Update composer dependencies (#1666) * Rearranging tests out of application/ into tests/ * Update composer packages to recent versions - Update PHPUnit - Update Behat - Update PHPSpec - Update aura/di to remove custom fork - Switch flysystem to 1.0 not 1.1 dev - Update phinx - Update phpunit tests for new version - Update behat tests for new version * Fix DI to work without auto resolving enabled * Run codeship builds on PHP 5.6 * Fix travis builds * Remove bin/tests * Fix phpunit tests failing while trying to backup globals Tests were failing with "Serialization of 'Closure' is not allowed" * Ignore tests/ for linting * fixup lint * Fix PHPUnit code coverage, and fix some bugs too * Don't inject Signer into webhook command Signer is just overwritten anyway, no need to inject it. Injection was broken because we didn't pass an auth token * Fix bin/ushahidi for new symfony/console version * Fixup coverage reporting from travis * Try to fix sending files to coveralls * Add PHP 7.1 to travis builds * Exclude php 7.1+coverage build * Add laravel/homestead box and remove puppet scripts * Update ohanzee/database to our fork, and select only aggregates when counting Mysql 5.7 doesn't like selecting both grouped and non grouped info in the same query. So we're not resetting the select fields before adding aggregates (ie. count) to the query * Remove posts?current_stage filter as its not working, and fails hard in mysql 5.7 * Set mysql strict mode in travis * Reset order by clause when fetching count queries too * Fix posts stats to work with mysql only_full_group_by mode * Output mysql version from travis builds * Use MAX() instead of ANY_VALUE() in queries ANY_VALUE only exists in mysql 5.7+ * Fix io scheduled job errors (#1748) * handle error in twitter dataprovider connection * empty filters are reverted to the default, if provided (helps avoid invalid SQL such as "IN ()") * Release 3.7.0 (#1764) * 1595 intercom (#1718) * adding intercom listener to capture user events * Adding additional intercom events * Adding first login * Fixing bracket * Adding intercom lib * Updating listener * Add laravel/homestead box and remove puppet scripts (#1722) * Updating composer * Updating composer.lock from fresh file * Fixing tests * Update users.feature * Remove redundant file * Fixed var name * Fix default for existing tasks * adding new migration to show_when_published (#1731) * Changing column default for task visibility * Repair linting * Fix Waffle badge Extra space was breaking markdown. * Adding total to geojson collection (#1739) * Adding total to geojson collection * Adding explanatory note about geojson addition * Adjusting unmapped request to incorporate it into stats (#1738) * Adjusting unmapped request to incorporate it into stats and changing the way in which unmapped is caluclated * Fixing failing test * Fixing get total count * Geojson chunking (#1744) * Adding total to geojson collection * Adding explanatory note about geojson addition * Removing unneeded logging * Fixed issue when no posts returned by search * Update linting setup and fix any new lint errors (#1742) * Update phpcs * Add ruleset for src/ * Fix linting on src/ * Lint tests * Make tests PSR4 compliant * Split Kohana bootstrap out of Feature Context so we can ignore it for CS * Fix spec ruleset * Fix linting on migrations * Fixing missing db config for unmapped query * Enable clustering by default * Only add `tags` type attributes to the "post fields" not tasks. * Remove category fields from tasks * Turn posts_tags into a full post value table, and avoid duplicating info - Add extra field to posts_tags - Remove tags from post_varchar - Add Post/Tags validator - Add Post/Tags repo * Don't allow making tags attributes private * Handle legacy tags values and save into first tags attribute. This isn't 100% compatible, but we at least save tags *if* the form has a tags attribute * Make forms tests work in isolation * Stop limiting tags by form, just limit tags per attribute - Stop using forms_tags table - Just use form_attribute.options instead * Rename tags to categories in error messages * Format form tags and tag children as relations, remove tag forms formatter * Remove tags?formId filter from Tag repo We've removed the relevant table, so lets remove the code too! * Fix validating tags when saving a post Typo when injecting tags_repo was breaking it * Fix validating tags when saving a post Typo when injecting tags_repo was breaking it * Add TagRepository interface to use in Post/Tags Validator - Use tag field to set tags in test - Add TagRepository Interface - Implement interface in Tag Repo * Ignore form.tags if passed when saving Form entities Ignore computed attributes on form * Add test for removing tag from attribute options when deleted * Test that form.tags value is ignored during form update * Make child tags top level when parent is deleted, don't delete them Update foreign key to ON DELETE SET NULL, not CASCADE * Reset fixtures before testing removing a tag Fix to c55dbfe * Parse tag names when set as post value (not just post.tags) Parse tags as name or id when sent in post.values. We already do this with post.tags * Update to use checklist for testing * Fixing error message for tag (#1760) * Fixing error message for tag * adding name to error-message when importing posts with categories * removing children from tag-object before updating * removing tags from form-object before saving form * Incorrect length requirement
This pull request makes the following changes:
Test these changes by:
Fixes #1595
Ping @ushahidi/platform
This change is