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

Fix csv download bug for users and refactor and functional tests #5257

Merged
merged 3 commits into from
Mar 29, 2019
Merged

Fix csv download bug for users and refactor and functional tests #5257

merged 3 commits into from
Mar 29, 2019

Conversation

cesswairimu
Copy link
Collaborator

@cesswairimu cesswairimu commented Mar 25, 2019 •
edited
Loading

Fixes #5255 and #5264

  • PR is descriptively titled 📑 and links the original issue above 🔗
  • tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with rake test
  • code is in uniquely-named feature branch and has no merge conflicts 📁
  • screenshots/GIFs are attached 📎 in case of UI updation
  • ask @publiclab/reviewers for help, in a comment below

@cesswairimu
Copy link
Collaborator Author

users_csv

@plotsbot
Copy link
Collaborator

plotsbot commented Mar 25, 2019 •
edited
Loading

1 Message
📖 @cesswairimu Thank you for your pull request! I’m here to help with some tips and recommendations. Please take a look at the list provided and help us review and accept your contribution! And don’t be discouraged if you see errors – we’re here to help.

Generated by 🚫 Danger

@cesswairimu cesswairimu changed the title Add to_csv method in the users model (wip)Add to_csv method in the users model Mar 25, 2019
@cesswairimu cesswairimu changed the title (wip)Add to_csv method in the users model (WIP)Add to_csv method in the users model Mar 25, 2019
@jywarren
Copy link
Member

Hi @cesswairimu -- what do you think is up with CodeClimate saying there are similar blocks of code? But they're for different models, right? Can we ignore?

https://codeclimate.com/github/publiclab/plots2/pull/5257

Thank you!!!

@cesswairimu
Copy link
Collaborator Author

Hey @jywarren I am looking into refactoring this also working on tests of the same as you suggested. Thanks

@cesswairimu cesswairimu force-pushed the fix-users-csv-download branch 5 times, most recently from b2b05fc to 67d7bc1 Compare March 25, 2019 23:10
@cesswairimu cesswairimu changed the title (WIP)Add to_csv method in the users model Fix csv download bug for users and refactor Mar 26, 2019
@cesswairimu
Copy link
Collaborator Author

@jywarren got carried away with the refactoring......refactored the controller too 😄 Its hard to add tests and run them since I am still experiencing #5207 this with my test suite. Created a follow-up issue for this here #5264 as soon as I fix this error locally will implement the tests. Thanks

@cesswairimu cesswairimu force-pushed the fix-users-csv-download branch from 67d7bc1 to 0b62e12 Compare March 27, 2019 20:03
@cesswairimu cesswairimu changed the title Fix csv download bug for users and refactor Fix csv download bug for users and refactor and functional tests Mar 27, 2019
@cesswairimu cesswairimu requested a review from jywarren March 27, 2019 21:59
@cesswairimu
Copy link
Collaborator Author

@jywarren implemented tests on this...kindly take a look

@cesswairimu cesswairimu requested a review from grvsachdeva March 28, 2019 16:36
@jywarren
Copy link
Member

This looks awesome @cesswairimu thank you! Sorry when I am a bit slow -- thanks for requesting a review from someone else -- a good idea! I just read through and this looks super!

@jywarren jywarren merged commit e7d1315 into publiclab:master Mar 29, 2019
@cesswairimu cesswairimu mentioned this pull request Mar 29, 2019
Closed
icarito pushed a commit to icarito/plots2 that referenced this pull request Apr 9, 2019
@cesswairimu @icarito
Fix csv download bug for users and refactor and functional tests (pub…
ebd6bf7 
…liclab#5257)

* Extract to_csv method in a module

* refactor stats controller

* add stats download tests
SrinandanPai pushed a commit to SrinandanPai/plots2 that referenced this pull request May 5, 2019
@cesswairimu @jywarren
Fix csv download bug for users and refactor and functional tests (pub…
903e8b2 
…liclab#5257)

* Extract to_csv method in a module

* refactor stats controller

* add stats download tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jywarren jywarren Awaiting requested review from jywarren

@grvsachdeva grvsachdeva Awaiting requested review from grvsachdeva

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

Successfully merging this pull request may close these issues.
3 participants
@cesswairimu @plotsbot @jywarren