Implement questions answered vs questions asked #4256

cesswairimu · 2018-12-10T15:45:25Z

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
PR is descriptively titled
ask @publiclab/reviewers for help, in a comment below

We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!

If tests do fail, click on the red X to learn why by reading the logs.

Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software

Thanks!

plotsbot · 2018-12-10T15:57:07Z

	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

jywarren · 2018-12-10T16:46:54Z

Hi! Just a hint - we can ignore the CodeClimate things, but sometimes they're really helpful suggestions. You can read them in detail and how to resolve them here: https://codeclimate.com/github/publiclab/plots2/pull/4256

jywarren · 2018-12-10T16:48:01Z

app/controllers/questions_controller.rb

  public

  def index
+    if params[:period]


This might be a good one to cache daily! I think esp. with collecting a year's worth of content each page load.

great! thanks

cesswairimu · 2018-12-12T19:54:05Z

cesswairimu · 2018-12-13T07:35:45Z

Hey @gauravano @SidharthBansal @jywarren Kindly take a look and any suggestions on how I can best cache the stats here would be great. Thanks.

grvsachdeva · 2018-12-13T23:46:12Z

As @jywarren mentioned we could cache for a day #4256 (comment)

we also cache home page data so you can see home_controller for reference. Let me know if you need help. Thanks!

cesswairimu · 2018-12-14T13:03:06Z

Hey @jywarren I did some refactoring and removed the whole bunch of instance variables. Does the caching I did on the questions helper method make sense? Thanks

jywarren · 2018-12-14T22:51:35Z

app/helpers/questions_helper.rb

+    SORTING_OPTIONS
+  end
+
+  def cache_quiz_stats


This works, but then people coding do have to remember to use the "cached version". Another way would be to build this inside the filtering() method itself, so all calls to that are filtered. If people really want the non-filtered version, they can manually flush it with... i forget but something like Rails.cache.flush(...). Make sense? This is exciting!

Yes it does thanks, on it

cesswairimu · 2018-12-16T18:10:59Z

@jywarren this is ready @publiclab/reviewers this is ready kindly review

jywarren

Thanks Cess, great work! I don't think we've as extensively used helpers before but I don't see a reason not to. Where do you think the bulk of statistics code would eventually live?

jywarren · 2018-12-18T16:36:52Z

app/helpers/questions_helper.rb

+module QuestionsHelper
+  SORTING_OPTIONS = %w(All Week Month Year).freeze
+
+  def filtering(period)


This looks good but perhaps it could have a more descriptive name -- something describing how it's related to stats, or Q&A? Thanks!

Also, I'd love to see a simple test for these methods. Where would that live?

Let me try and figure out where the tests will go. Thanks

jywarren · 2018-12-18T16:37:51Z

app/helpers/questions_helper.rb

+
+    if period == 'All'
+      Rails.cache.fetch("all_stats", expires_in: 1.days) do
+        @asked = Node.questions.to_a.size


I think if you use Node.questions.count it should do a more efficient query, just a SQL SIZE query instead of fetching all the full records and counting them. Maybe same on line below! 👍

@jywarren tried .count was not working fine because its a hash returning results like so {82=>1, 85=>2, 86=>2, 87=>1}. Using .length instead. hope that's a better query?

jywarren · 2018-12-18T16:38:19Z

app/views/questions/index.html.erb

+        </div>
+        <div class="col-md-4">
+            <%= form_tag request.url, :method => 'get'  do %>
+                <%= select_tag :period, options_for_select(options),onchange: "this.form.submit();", include_blank: "Filter stats", class: "form-control"%>


Ooh this is cool, could you share a screenshot? Thanks!

@jywarren a screenshot is ☝️ above. Thanks for the feedback.

jywarren · 2018-12-18T16:40:33Z

Regarding the question of where statistics code "home base" is - would it fit in a model, so it's testable via unit tests? But open to ideas and brainstorming. Maybe ask @gauravano or @SidharthBansal for input on this as well!

cesswairimu · 2018-12-20T17:33:53Z

@jywarren I implemented the tests and changed the method name to a better name and improved the queries. Please take a look.

cesswairimu · 2018-12-20T17:34:40Z

Regarding the question of where statistics code "home base" is - would it fit in a model, so it's testable via unit tests? But open to ideas and brainstorming. Maybe ask @gauravano or @SidharthBansal for input on this as well!

@jywarren is this for this ticket #4317?

jywarren · 2018-12-22T02:17:13Z

Actually it's more broad - that is, should we have a central location where we keep much of the statistics code, as if in a model, or should we be OK with some appearing in controllers, some in helpers... i'm just trying to think critically about how we're doing overall code organization. We don't have to come up with a final solution right now! Thanks!

jywarren · 2018-12-22T02:17:50Z

This is great. Merging!!!

* implement questions answered vs questions asked * Refactoring the questions stats * caching questions stats * test question helper

ghost assigned cesswairimu Dec 10, 2018

ghost added the in progress label Dec 10, 2018

jywarren reviewed Dec 10, 2018

View reviewed changes

cesswairimu force-pushed the stats-on-questions branch 7 times, most recently from 946d3f0 to 0bdc26a Compare December 12, 2018 19:38

cesswairimu force-pushed the stats-on-questions branch 3 times, most recently from dc417cf to 2d477d9 Compare December 13, 2018 07:33

jywarren reviewed Dec 14, 2018

View reviewed changes

cesswairimu force-pushed the stats-on-questions branch from 2d477d9 to be99757 Compare December 16, 2018 17:48

cesswairimu changed the title ~~(WIP) Implement questions answered vs questions asked~~ Implement questions answered vs questions asked Dec 16, 2018

cesswairimu added 3 commits December 16, 2018 22:32

implement questions answered vs questions asked

22e45b8

Refactoring the questions stats

c0db8a9

caching questions stats

715ccd4

cesswairimu force-pushed the stats-on-questions branch from be99757 to 715ccd4 Compare December 16, 2018 19:32

jywarren requested changes Dec 18, 2018

View reviewed changes

cesswairimu force-pushed the stats-on-questions branch from 07418b5 to a7e0cd7 Compare December 20, 2018 16:48

cesswairimu force-pushed the stats-on-questions branch 2 times, most recently from a8234a2 to cbf49d1 Compare December 20, 2018 16:52

test question helper

e9fd267

cesswairimu force-pushed the stats-on-questions branch from cbf49d1 to e9fd267 Compare December 20, 2018 17:26

jywarren merged commit 1802e62 into publiclab:master Dec 22, 2018

ghost removed the in progress label Dec 22, 2018

cesswairimu mentioned this pull request Nov 17, 2019

Calculate "answered" rate for questions posted in a certain time period, by tag #2120

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement questions answered vs questions asked #4256

Implement questions answered vs questions asked #4256

cesswairimu commented Dec 10, 2018 •

edited

Loading

plotsbot commented Dec 10, 2018 •

edited

Loading

jywarren commented Dec 10, 2018

jywarren Dec 10, 2018

cesswairimu Dec 10, 2018

cesswairimu commented Dec 12, 2018

cesswairimu commented Dec 13, 2018 •

edited

Loading

grvsachdeva commented Dec 13, 2018

cesswairimu commented Dec 14, 2018 •

edited

Loading

jywarren Dec 14, 2018

cesswairimu Dec 14, 2018

cesswairimu commented Dec 16, 2018 •

edited

Loading

jywarren left a comment

jywarren Dec 18, 2018

jywarren Dec 18, 2018

cesswairimu Dec 18, 2018

jywarren Dec 18, 2018

cesswairimu Dec 18, 2018

jywarren Dec 18, 2018

cesswairimu Dec 18, 2018

jywarren commented Dec 18, 2018

cesswairimu commented Dec 20, 2018

cesswairimu commented Dec 20, 2018

jywarren commented Dec 22, 2018

jywarren commented Dec 22, 2018

Implement questions answered vs questions asked #4256

Implement questions answered vs questions asked #4256

Conversation

cesswairimu commented Dec 10, 2018 • edited Loading

plotsbot commented Dec 10, 2018 • edited Loading

jywarren commented Dec 10, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cesswairimu commented Dec 12, 2018

cesswairimu commented Dec 13, 2018 • edited Loading

grvsachdeva commented Dec 13, 2018

cesswairimu commented Dec 14, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cesswairimu commented Dec 16, 2018 • edited Loading

jywarren left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jywarren commented Dec 18, 2018

cesswairimu commented Dec 20, 2018

cesswairimu commented Dec 20, 2018

jywarren commented Dec 22, 2018

jywarren commented Dec 22, 2018

cesswairimu commented Dec 10, 2018 •

edited

Loading

plotsbot commented Dec 10, 2018 •

edited

Loading

cesswairimu commented Dec 13, 2018 •

edited

Loading

cesswairimu commented Dec 14, 2018 •

edited

Loading

cesswairimu commented Dec 16, 2018 •

edited

Loading