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

Fix Bothub classifier and add tests #196

Merged
merged 5 commits into from
Nov 4, 2019

Conversation

matmsa27
Copy link
Contributor

@matmsa27 matmsa27 commented Nov 4, 2019

Hi @nicpottier , we fix it the ClassifierTypeBothub for bh and add BotHub type on the test list for run tests.

Thanks a lot.
Regards

@nicpottier
Copy link
Member

Thanks, ya, that's kind of a shame, we are using a longer type for classifier types since that makes it easier for everyone. Why don't we change the type on the Bothub type to bothub on the RapidPro side. (I'm fine with that PR and this one in adding that) A quick DB shell can be used to update any classifiers already created but at least will keep things in sync.

@matmsa27
Copy link
Contributor Author

matmsa27 commented Nov 4, 2019

Hmmm, yeah make sense. I opened a PR for RapidPro here. I will change classifier_type on mailroom_db.py to bothub, and I'll change slug on classifiers/types/bothub/type.py for bothub.

Here in this PR I'll remove the change for bh and keep bothub, and generate a new mailroom_test.dump.

Can I proceed with these observations?
Thanks a lot.

@codecov
Copy link

codecov bot commented Nov 4, 2019

Codecov Report

Merging #196 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #196   +/-   ##
=======================================
  Coverage   47.08%   47.08%           
=======================================
  Files          79       79           
  Lines        7038     7038           
=======================================
  Hits         3314     3314           
  Misses       3190     3190           
  Partials      534      534
Impacted Files Coverage Δ
models/classifiers.go 41.37% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1f69489...336f9ca. Read the comment docs.

@nicpottier
Copy link
Member

Yep, that'd be perfect!

@nicpottier nicpottier merged commit 476fba2 into rapidpro:master Nov 4, 2019
@nicpottier
Copy link
Member

Thanks guys!

@matmsa27 matmsa27 deleted the feature/add_bothub_to_tests branch June 22, 2020 19:41
rasoro pushed a commit to Ilhasoft/mailroom that referenced this pull request Mar 10, 2023
rasoro pushed a commit to Ilhasoft/mailroom that referenced this pull request Dec 16, 2024
Email service construction should back down to parent org SMTP settings
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.

3 participants