Fixes #11331

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

• [x] 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
• [x] code is in uniquely-named feature branch and has no merge conflicts 📁
• [ ] screenshots/GIFs are attached 📎 in case of UI updation
• [x] ask @publiclab/reviewers for help, in a comment below

Code Climate has analyzed commit 8ae260fa and detected 4 issues on this pull request.

Here's the issue category breakdown:

Category	Count
Style	4

View more on Code Climate.

This pull request generated screenshots of many common pages in the running app. You should be able to download and view them here: https://github.com/publiclab/plots2/suites/7689226168/artifacts/321116664

Codecov Report

Merging #11332 (73ab354) into main (8b97a11) will increase coverage by 0.09%. The diff coverage is 100.00%.

:exclamation: Current head 73ab354 differs from pull request most recent head 1850bf3. Consider uploading reports for the commit 1850bf3 to get more accurate results

@@            Coverage Diff             @@
##             main   #11332      +/-   ##
==========================================
+ Coverage   82.46%   82.56%   +0.09%     
==========================================
  Files          98       96       -2     
  Lines        5995     5960      -35     
==========================================
- Hits         4944     4921      -23     
+ Misses       1051     1039      -12     

Impacted Files	Coverage Δ
app/controllers/wiki_controller.rb	82.97% <100.00%> (+0.12%)	:arrow_up:
app/models/tag.rb	93.60% <100.00%> (+0.30%)	:arrow_up:
app/controllers/home_controller.rb	95.77% <0.00%> (-0.18%)	:arrow_down:
app/models/user.rb	86.49% <0.00%> (-0.15%)	:arrow_down:
app/models/node.rb	91.18% <0.00%> (-0.09%)	:arrow_down:
app/models/comment.rb	77.70% <0.00%> (-0.08%)	:arrow_down:
app/models/answer.rb
app/models/drupal_content_field_image_gallery.rb
app/controllers/comment_controller.rb	83.07% <0.00%> (+0.26%)	:arrow_up:
app/controllers/map_controller.rb	50.33% <0.00%> (+0.33%)	:arrow_up:
... and 1 more

Ah I believe it's using a private method from the bottom of the previous controller. You could maybe move that method into the application controller, which would make it available to all controller methods!

On Mon, Aug 8, 2022, 5:36 PM Karishma Vanwari @.***> wrote:

@.**** commented on this pull request.

In app/controllers/wiki_controller.rb https://github.com/publiclab/plots2/pull/11332#discussion_r940684415:

@@ -60,6 +60,11 @@ def show if @.***? # it's a place page! @tags = @node.tags @tags += [Tag.find_by(name: params[:id])] if Tag.find_by(name: params[:id])

•  tag1, tag2 = @node.normal_tags(:followers).includes(:tag).pluck(:name).first(2)
  

•  # get recommendations
  

•  @recommendations = Tag.get_recommendations(tag1, tag2)
  

I tried placing this piece of code on line 54 in home_conroller but it gives the following error - NoMethodError in HomeController#dashboard_v2 undefined method normal_tags for nil:NilClass .

@TildaDares https://github.com/TildaDares @jywarren https://github.com/jywarren

— Reply to this email directly, view it on GitHub https://github.com/publiclab/plots2/pull/11332#discussion_r940684415, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAF6J724O7ZGEHNGYFPYTTVYF4ULANCNFSM55XCV65Q . You are receiving this because you were mentioned.Message ID: @.***>

Hi @jywarren, where do I need to shift the method since when I shift the dashboard_v2 method in application_controller, it gives an error saying The action dashboard_v2 could not be found for HomeController? So maybe I need to add it in an already existing method in application_controlller, but which one -- how do I figure it out?

Just commenting here with notes from our call before, suggested pseudocode for making remote calls to open sub-topics:

Tag…..each do |subtopic|

    <script>
      $(‘#expand-btn-<%= index %>’).click(function(e) {
        $.ajax(‘/tags/subtopics?subtopic=<%= subtopic %>’).done(function(response) {
            $(‘#contents-UNIQ’).append(response.responseText);
        });
      });
    </script>

end

Controller:

def subtopics
  Tag.find…… params[:subtopic]
  render “tags/subtopic”, layout: false
end

Hi @KarishmaVanwari -- why don't we start by making the subtopics method:

def subtopics
  Tag.find…… params[:subtopic]
  render “tags/subtopic”, layout: false
end

We'll need an entry in config/routes.rb to point a URL at this method too. Then we can merge that PR and then build on that.

Let's open that PR first, what do you say? I'll look more at this one and we'll narrow its scope as well, and it can work with that one.

Hi @jywarren, I've opened a PR #11380. Thanks!

My apologies for mixing these all up, but i hid the subtopics discussion from this thread to try to clear it up. Looking at the issues here now!

OK, so I had to make some edits to the dashboard code to make it work, but ultimately I found that Tag.get_recommendations("blog", "three") is not returning anything (i used "blog" because that's in the db seeds file so it exists).

So let's debug why that's not working. I think we may want to write a unit test for that code. That will let us test Tag.get_recommendations("blog", "three") in isolation and ensure it returns the right thing. Why don't I write the unit test and you can work on figuring out why it's not returning anything!

This pull request generated screenshots of many common pages in the running app. You should be able to download and view them here: https://github.com/publiclab/plots2/suites/8142454649/artifacts/353100585

Generally unit tests test individual functions in isolation, not in the context of a big running application all at once. They're useful for finding narrow reasons things may have gone wrong, and that individual pieces function as expected.

By contrast, integration tests, or system tests, are "full-stack" tests which start up the whole application and help us see how things might go wrong when everything is trying to work in synchrony, all at once.

Both types of test -- the narrow ones and the broad ones -- are useful for catching errors. Sometimes both get tripped! But I hope that way of thinking of different kinds of tests makes sense!

This pull request generated screenshots of many common pages in the running app. You should be able to download and view them here: https://github.com/publiclab/plots2/suites/8164595320/artifacts/354647543

Hi :smile:, this issue has been automatically marked as stale because it has not had recent activity. Don't worry you can continue to work on this and ask @publiclab/reviewers to add in-progress label :tada: . Otherwise, it will be closed if no further activity occurs in 10 days -- but you can always re-open it if you like! :100: Thank you for your contributions! :raised_hands: :balloon:.