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

Grant new users formbuilder perms #893

Merged
merged 5 commits into from
Feb 1, 2017

Conversation

ivermac
Copy link
Contributor

@ivermac ivermac commented Jan 27, 2017

closes #892

@ivermac
Copy link
Contributor Author

ivermac commented Jan 27, 2017

I can't seem to find a better way to test a post_save signal - there aren't any (or I haven't seen any) signal tests on the repo. Using @patch decorator on this test case and checking if the mock was called e.g. self.assertTrue(mock.called) failed for me - returned false. If anyone has a better way to handle this, please mention it here.

@ivermac ivermac changed the title 892 grant new users formbuilder perms Grant new users formbuilder perms Jan 27, 2017
@ivermac ivermac requested a review from denniswambua January 27, 2017 11:45
@skambo skambo self-assigned this Jan 30, 2017
@skambo
Copy link
Contributor

skambo commented Jan 30, 2017

@ivermac on stage, after creating a new acc, the error is still there

screen shot 2017-01-30 at 12 31 10 pm

@skambo skambo added the QA- PR failed QA testing label Jan 30, 2017
@skambo
Copy link
Contributor

skambo commented Jan 30, 2017

@ivermac @denniswambua It works now for new users

@skambo skambo added QA+ PR passed QA testing and removed QA- PR failed QA testing labels Jan 30, 2017
@skambo skambo removed their assignment Jan 30, 2017
Copy link
Member

@pld pld left a comment

Choose a reason for hiding this comment

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

testable?

@ivermac
Copy link
Contributor Author

ivermac commented Jan 31, 2017

See the second comment on the PR

Copy link
Contributor

@denniswambua denniswambua left a comment

Choose a reason for hiding this comment

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

@ivermac
Copy link
Contributor Author

ivermac commented Jan 31, 2017

I think what should be tested is whether the signal function was called or not.

@pld
Copy link
Member

pld commented Jan 31, 2017 via email

@denniswambua denniswambua force-pushed the 892-grant-new-users-formbuilder-perms branch from 73ba0b0 to fbdc902 Compare February 1, 2017 07:23
@denniswambua denniswambua merged commit dff374e into master Feb 1, 2017
@denniswambua denniswambua deleted the 892-grant-new-users-formbuilder-perms branch February 1, 2017 10:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
QA+ PR passed QA testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Grant formbuilder object permissions on creation of a user profile
4 participants