-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Send reports via SMS #2708
Comments
See the following for phone number formatting: http://www.mediaburst.co.uk/blog/convert-a-mobile-number-into-international-format/ |
Does this feature really make sense in times of smartphones / email / analytic apps / nearby constant wifi / EDGE / 3g / 4g connectivity? IMO, sending a SMS would absolutely make sense in context of #1486 (any alert is triggered for a website), because an alert often indicates that urgent action is required. |
I'd rather see Alerts and push notifications, but we don't dictate what others work on here. The only thing we should decide is whether this is included in the core distribution or not. |
I think it makes sense because not all the world is connected like EU/North america is (3g, constant wifi do not exist in most places in the world). SMS is huge is Asia, Africa, south america, etc. Also, this ticket will build all the SMS logic handling code which will then be reused for Custom Alerts #1486 where, as mentionned, it will be even more interesting and useful. It will be included in the core but disabled by default. |
I don;t think this is stored per user. Instead it is stored as a general Option value. Otherwise, proposal looks great, looking forward to seeing it! |
Maybe you want to use something fancy for the country selector,see this cool jquery plugin: http://baymard.com/labs/country-selector Btw for the country selector, please use existing country translations. Try to default to the country of the selected language. Later, we might reuse this website selector in other places :) |
Now that plugins can have their own config files (#2871), I was wondering what is the best solution to store the phone numbers and SMS API tokens for the SMSManager plugin. Do they belong in the database in the option table or in the new local config file ? Is it only a question of being able to replicate this data easily when using multiple Piwik instances or is there other factors to weight-in ? |
Yes, it depends. In this case, I would say database because:
|
+1 with database storage of phone numbers and tokens, in the "piwik_option" table. |
There's a very high probability this plugin will be used to send alerts. Could you please confirm 'SMSManager' is the name that makes the most sense? |
Updated report description for more clarity + Proposal to use MultiSites.getAll and a new MultiSites.get to fetch the data to use in the SMS reports. The advantage of doing that would be to not have to process the % evolution in the SMS manager implementation. it would be better to leave all data processing to the MultiSites.get* API. Thoughts? |
We currently have one type of report : e-mail. We now would like to implement a new type : sms. While doing so, we would like to refactor the existing PDFReport plugin so it can manage both types, and possibly more in the future. The idea is to use the hooking/event mechanism to allow other plugins to add new types of reports. We already know that e-mail and sms reports share some properties but they do also have specifics ones. shared properties :
e-mail specific properties :
The first question we need to answer is how do we manage persistence. There are many known academic anwsers on how to store polymorphic objects in a relational database.
Here are two less academic ways to solve this issue :
|
The JSON parameters array seems a good idea because it doesn't add a new table.
It's not obvious what the best solution is... Because it's quite easy to add new fields on Piwik update, the "one table with nullable fields " is also do-able... I think I would implement it with the JSON solution still, thoughts? |
(In [6478]) fixes #2708 refs #5491
|
Julien, Wonderful patch! Thank you for this work. Really nice refactoring and well written code. Code review
I think this would be caused by the revenue beautifying code in Piwik::getPrettyValue - it shouldn't be called on money when metric contains 'evolution' ;)
/index.php?module=API&action=index&idSite=1&period=day&date=yesterday&updated=1&token_auth=0b809661490d605bfd77f57ed11f0b14&method=PDFReports.generateReport&idReport=2&outputType=1&language=en Output= Fatal error: Call to a member function getColumns() on a non-object in piwik\svn\trunk\core\API\ResponseBuilder.php on line 176
The <?XML response shouldn't be in the .sms output I believe this is a bug?
It looks good - we used GLOBALS but could have used the static class solution instead.
Api credentials and phone numbers should be removed when the plugin is deleted, not really when it's just deactivated. There is a method uninstall() on the Plugin class, but currently the UI does not allow to delete a plugin. So I think it's better to not delete API credentials on deactivate.
It would be great to improve the UI. I have some ideas see below. Also will have more once the UI works for me (see below for the bug i experience preventing signing in mediaburst)
Ideally yes it should be in that file, but we haven't been enforcing this greatly.
Testing
Add new DIV to describe the SMS provider' I propose that we add a text such as the following:
This text could be returned as a getter on the provider class. Looking forward to seeing further small changes suggested in this ticket! As a conclusion, I will say that this is a great new feature(s), and again Big Kuddos to you for the vision and the very nice implementation!! |
(In [6563]) Refs #2708 Small changes |
(In [6614]) Refs #2708 Using Piwik_Common::json* instead of core PHP to avoid annoying PHP bugs with old php builds |
UI Review
Phone numbers admin
After adding a phone number, write "We just sent a SMS to this number with a code: please enter this code below and click "Validate"."
Edit Report UI
|
More suggestions/report:
|
(In [6727]) refs #2708
TODO
|
(In [6728]) refs #2708 fixing integration tests failing after r6727 |
Very cool, thanks for doing all changes! :) Feedback
I will also commit a few small changes |
(In [6738]) Refs #2708
|
(In [6741]) Refs #2708, tweak english translations. |
(In [6758]) refs #2708
|
(In [6761]) refs #2708 fixing regression introduced in r6758 |
I noticed an inefficiency in the Report generation. I notice that when I run the following SMS "Single site" report with debug enabled: with, in the config file:
It outputs on screen: http://pastebin.com/8TyxhpAr As you can see when generating one SMS reports it first loads all archives from the DB for ALL websites. The SMS was a "single website" so it should only query this site data. I havent looked at the code please let me know if you need help or more info! :) NB: I tried generating a PDF that didn't contain the "All websites dashboard" and it does not do the extra requests for other websites, so maybe it's a problem with SMS only? |
(In [6764]) refs #2708 blacklisting MobileMessaging plugin from API integration tests |
(In [6772]) Refs #2708
|
integration tests task has been logged in #3323 |
using POST instead of GET to send SMS logged in #3325 |
In the future, I think it would be cool to ask during the installation process if users want to send SMS reports, with a checkbox, and if so enable the MobileMessaging plugin, to maximise number of users who know about this feature. Kuddos Julien for this great new feature! |
list of improvements logged in #3337 |
(In [6849]) refs #3323 #3088 #2708 #71 #2318
|
The idea is to send reports via SMS using online SMS providers. Users need to provide one or several phone numbers and a SMS authorization token.
Reports
So far, we have identified three different kinds of reports. Here are the candidate templates :
Desirable features
SMS Authorization Token and Account Management
Online SMS providers with open APIs provide a SMS authorization token to registered users.
Within Piwik, this token will be manageable one of two ways :
In this case, it is the responsibility of the super user to provide the token in the SMS management user interface.
It is the only user who has the right to consult the number of remaining credits.
In this case, it is the responsibility of each user to provide the token.
Each user has access to the number of remaining credits.
Deciding between the two modes of management is done by the super user with the following options in the SMS management UI :
The second option is the default.
When a token is typed-in by the user, a call to the SMS API will be performed to validate the token and display the amount of remaining credit. The token will not be stored if it can't be validated.
The user will have the ability to display the number of remaining credits in the SMS management pages. A message will be displayed in the report and SMS management pages when no credit are left in the account.
The token can not be modified. To change an already validated token the user will have to remove it and add a new one.
If the token is not provided
When the token is managed by the super user and a standard user accesses the SMS management page, display "SMS reports are not properly configured, please contact your administrator".
Phone Number
Within the SMS management UI, each user can manage one or several phone numbers. A phone number will be selectable within the report creation and edition forms only if it has been validated. To validate a phone number, a token is sent via SMS after requesting it in the SMS management page. This token is then typed-in by the user.
Here is the candidate SMS validation template :
The validation token does not expire.
Phone numbers can not be modified. To change an already validated number the user will have to remove it and add a new one.
The UI will provide a list of countries to automatically assign the international extension number. The country corresponding to the current language will be selected as the default value.
The phone number input field accepts only numbers.
A report can be created and edited without assigning phone numbers. The report list will display a warning message and the user won't be able to send the report.
In the report page, when the user has not validated at least one number :
SMS reports are still editable, the form can be saved without a selected phone number but the following message will be displayed in the edit form: "The list of phone numbers is empty because you did not activate a phone number". A link will be provided for quick access.
SMS Management Page
The SMS management page will be added in "Settings".
Technical aspects
The text was updated successfully, but these errors were encountered: