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

Junebug channel for RapidPro. #435

Merged

Conversation

smn
Copy link
Contributor

@smn smn commented Jan 30, 2017

I've taken the Jasmin channel as an example and have modelled the Junebug channel along the same lines.

As Junebug can support both SMS and USSD in the same channel type I suspect more work is required here to get this to work as a USSD channel in RapidPro.

This should be enough to get it to work with SMS.

.claim-description
%div
-blocktrans
Connect your <a href="http://junebug.praekelt.org/" target="_blank">Junebug</a> instance that you have

Choose a reason for hiding this comment

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

Should we not be prompting https here just for good measure?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 2ebf46f on praekeltfoundation:feature/junebug-channel into ** on rapidpro:master**.

external_id=data['message_id'])
return HttpResponse('OK')

else: # pragma: needs cover
Copy link
Contributor

Choose a reason for hiding this comment

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

  • 'needs cover' is for existing code that we haven't yet written tests for so that we can keep coverage at '100%' - makes it easier to see where coverage has changed on a PR.
  • 'no cover' is for stuff those few odd things we're ok with not testing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I'll remove these and attend to the coverage. I suspect this is due to me following the Jasmin example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've fixed these with the last commit, this last one I've removed entirely as invalid actions are currently guarded against by the urlconf pattern for the handler.

@@ -15,6 +15,7 @@ install:
- pip install --upgrade pip
- pip --version
- pip install -r pip-freeze.txt --upgrade
- pip install setuptools==33.1.1 # Recent version breaks packaging.requirements, this is a temp fix: https:/pypa/setuptools/issues/942
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you seen this fail on Travis? pypa/setuptools#942 (comment) Seems to suggest we should be ok with latest pip here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it was failing last week when I wrote this, I'll remove it again as hopefully it's been fixed now. Thanks for spotting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Still failing on Travis, I've put this back for now.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 95d0764 on praekeltfoundation:feature/junebug-channel into ** on rapidpro:master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling b16081c on praekeltfoundation:feature/junebug-channel into ** on rapidpro:master**.

@smn
Copy link
Contributor Author

smn commented Jan 30, 2017

@rowanseymour thanks for the initial review, should be ready for another pass.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 6f0b2a2 on praekeltfoundation:feature/junebug-channel into ** on rapidpro:master**.

@smn
Copy link
Contributor Author

smn commented Jan 30, 2017

Thanks! 🍰

Will you merge and if not, please let me know what else is expected from me :)

@nicpottier
Copy link
Member

nicpottier commented Jan 30, 2017 via email

@smn
Copy link
Contributor Author

smn commented Jan 30, 2017

Cool, working on this internally.

@smn
Copy link
Contributor Author

smn commented Jan 31, 2017

Submitted via email

@@ -0,0 +1,20 @@
# -*- coding: utf-8 -*-
Copy link
Member

Choose a reason for hiding this comment

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

Just merged from us because the migration squashing wasn't on /rapidpro yet and redoing that wasn't in the cards. Shouldn't affect this PR except that this migration should be 0058 and depend on 0057

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.008%) to 99.992% when pulling 6df71e1 on praekeltfoundation:feature/junebug-channel into e293144 on rapidpro:master.

@smn
Copy link
Contributor Author

smn commented Feb 1, 2017

I'll look into the coverage drop.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling ef139d4 on praekeltfoundation:feature/junebug-channel into e293144 on rapidpro:master.

@smn
Copy link
Contributor Author

smn commented Feb 1, 2017

@nicpottier changes applied, ready for re-review.

@nicpottier
Copy link
Member

nicpottier commented Feb 1, 2017

Great, will merge this today. Does Junebug have a squarish logo we can use on the channel claim page? If so I'll add this to the font icon set so it can be all pretty-like. (need something square in a vector format, svg would be best)

@smn
Copy link
Contributor Author

smn commented Feb 1, 2017

We've got https:/praekelt/junebug-marketing/blob/master/html/img/junebug-logo.svg but that's not quite square. I'm checking to see if we have something square.

@nicpottier
Copy link
Member

Nice ya that would work if maybe we just had the Jun squiggle piece?

@nicpottier nicpottier merged commit e1229b4 into rapidpro:master Feb 1, 2017
@nicpottier
Copy link
Member

Merged this just to get it in (since we are moving super quick right now) but would still love the squiggle and will work that in if you get it to me!

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.

5 participants