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.

Sign up for GitHub

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

Jump to bottom

Update Enketo kwargs arguments #1825

Merged
merged 2 commits into from
May 13, 2020
Merged

Update Enketo kwargs arguments #1825

merged 2 commits into from
May 13, 2020

Conversation

WinnyTroy
Copy link
Contributor

closes #1824

@WinnyTroy WinnyTroy requested review from DavisRayM and ukanga May 7, 2020 06:46
@DavisRayM
Copy link
Contributor

DavisRayM commented May 7, 2020 •
edited
Loading

Could we please also add/update tests for this scenario(if possible)... A bit scary that we didn't notice that passing form_pk there would not work.

Ideally the test would ensure anonymous users can make submissions via the enketo url

DavisRayM
DavisRayM reviewed May 8, 2020
View reviewed changes
onadata/apps/api/tests/viewsets/test_organization_profile_viewset.py
@@ -352,7 +352,7 @@ def test_member_sees_orgs_added_to(self):
request = self.factory.get('/', **self.extra)
response = view(request)
self.assertEqual(response.status_code, 200)
self.assertEqual(response.data, [expected_data])
self.assertEqual(dict(response.data[0]), expected_data)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did we have to change this ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah for some reason, the response_data returned an OrderedDict object.
I didnt see the correlation with the changes made however.

Copy link
Contributor

Choose a reason for hiding this comment

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

Mhh okay, was the test failing because of it ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

lemme confirm this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For clarification, this change occurs because as a result of a sorting issue and does not fail because of the changes to the enketo kwargs

@DavisRayM DavisRayM added the Blocks Release Issues that block the next release from being tagged. label May 8, 2020
@WinnyTroy WinnyTroy force-pushed the Update_enketo_kwargs branch 2 times, most recently from 130006b to e942a3b Compare May 8, 2020 12:00
@WinnyTroy WinnyTroy removed the Blocks Release Issues that block the next release from being tagged. label May 8, 2020
DavisRayM
DavisRayM previously approved these changes May 12, 2020
View reviewed changes
Copy link
Contributor

@DavisRayM DavisRayM left a comment

Choose a reason for hiding this comment

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

Please rebase with master @WinnyTroy

@DavisRayM DavisRayM merged commit ad71f66 into master May 13, 2020
@DavisRayM DavisRayM deleted the Update_enketo_kwargs branch May 13, 2020 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DavisRayM DavisRayM DavisRayM approved these changes

@ukanga ukanga Awaiting requested review from ukanga

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Enketo data submission requires login
2 participants
@WinnyTroy @DavisRayM