Remove "parent" in tag.rb #9404

jcads · 2021-03-31T04:19:06Z

Remove:

any instance of the word "parent" in tag.rb

plots2/app/controllers/tag_controller.rb

Lines 473 to 486 in ee46fe0

    
           def add_parent 
        
             if logged_in_as(['admin']) 
        
               @tag = Tag.find_by(name: params[:name]) 
        
               @tag.update_attribute('parent', params[:parent]) 
        
               if @tag.save 
        
                 flash[:notice] = "Tag parent added." 
        
               else 
        
                 flash[:error] = "There was an error adding a tag parent." 
        
               end 
        
               redirect_to '/tag/' + @tag.name + '?_=' + Time.now.to_i.to_s 
        
             else 
        
               flash[:error] = "Only admins may add tag parents." 
        
             end 
        
           end

plots2/config/routes.rb

Line 209 in c188b97

get 'tag/parent' => 'tag#add_parent'
plots2/app/controllers/tag_controller.rb

Line 3 in ee46fe0

before_action :require_user, only: %i(create delete add_parent)

Removing the above should cause the following tests to fail, so remove relevant tests in:

plots2/test/unit/node_tag_test.rb
plots2/test/functional/tag_controller_test.rb

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

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

gitpod-io · 2021-03-31T04:19:09Z

app/models/tag.rb

codecov · 2021-03-31T04:25:30Z

Codecov Report

❗ No coverage uploaded for pull request base (main@ebaad12). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head 2301891 differs from pull request most recent head cc8274a. Consider uploading reports for the commit cc8274a to get more accurate results

@@           Coverage Diff           @@
##             main    #9404   +/-   ##
=======================================
  Coverage        ?   58.44%           
=======================================
  Files           ?       98           
  Lines           ?     5985           
  Branches        ?        0           
=======================================
  Hits            ?     3498           
  Misses          ?     2487           
  Partials        ?        0

jcads · 2021-03-31T04:26:04Z

@jywarren kindly review if I made the correct changes in tag.rb. Thanks!

jywarren

Gosh, love seeing all this code go away!

I think i found a small mistake at before_action and suggested a fix. I hope that resolves the tests but we can look closer at what goes wrong in the tests if not!

Thank you very much and apologies for the slow review. Please feel free to highlight PRs that need review in the chatroom at https://gitter.im/publiclab/publiclab if we miss them, and thanks for your patience!!

jywarren · 2021-04-13T15:48:24Z

app/controllers/tag_controller.rb

@@ -1,6 +1,5 @@
 class TagController < ApplicationController
  respond_to :html, :xml, :json, :ics
-  before_action :require_user, only: %i(create delete add_parent)


Hi, actually here I believe we need to remove just the add_parent part, but keep the rest of the line!

Oh ok, got it!

app/controllers/tag_controller.rb

jywarren · 2021-04-13T15:56:03Z

app/models/tag.rb

    order = 'node_revisions.timestamp DESC'
    order = 'created DESC' if type == 'note'
-    Node.where('node.nid IN (?)', (nodes + parents).collect(&:nid))
+    Node.where('node.nid IN (?)', nodes.collect(&:nid))


Nicely done here! Actually we should be able to remove the comment on line 103 too, also related to parenting:

# .where('term_data.name IN (?) OR term_data.parent in (?)', tagnames, tagnames) # greedily fetch children

jywarren · 2021-04-13T15:56:18Z

app/models/tag.rb

@@ -342,13 +333,13 @@ def self.tagged_nodes_by_author(tagname, user_id)
    if tagname[-1..-1] == '*'
      @wildcard = true
      Node.includes(:node_tag, :tag)
-          .where('term_data.name LIKE(?) OR term_data.parent LIKE (?)', tagname[0..-2] + '%', tagname[0..-2] + '%')
+          .where('term_data.name LIKE(?)', tagname[0..-2] + '%')


Nice, so much simpler!

config/routes.rb

jcads · 2021-04-14T12:47:05Z

I don't know why there are conflicts though. It seems like it was just the code I removed.

RuthNjeri · 2021-04-15T19:21:04Z

@jcads, the conflict seems related to the tests https://github.com/publiclab/plots2/pull/9404/conflicts

codeclimate · 2021-04-16T02:23:41Z

Code Climate has analyzed commit cc8274a and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category	Count
Style	1

View more on Code Climate.

jywarren · 2021-04-17T04:39:36Z

Awesome work, thank you so much!!!!

* remove parent publiclab#9403 * remove "parent" * Fix test errors

remove parent publiclab#9403

23453a6

codeclimate bot reviewed Mar 31, 2021

View reviewed changes

app/models/tag.rb Outdated Show resolved Hide resolved

remove "parent"

b8a1b11

jcads force-pushed the migrate-new-tags branch from fe7394f to b8a1b11 Compare March 31, 2021 09:21

jywarren requested changes Apr 13, 2021

View reviewed changes

Fix test errors

909ffcb

jcads requested a review from jywarren April 14, 2021 12:44

Merge branch 'main' into migrate-new-tags

cc8274a

jywarren approved these changes Apr 17, 2021

View reviewed changes

jywarren merged commit aaeb5e6 into publiclab:main Apr 17, 2021

jcads deleted the migrate-new-tags branch April 17, 2021 07:27

This was referenced May 4, 2021

Remove instances of "parent" in tag.rb #9403

Closed

Revamp/deprecate tag aliasing, migrate aliases to new tags #6367

Open

reginaalyssa pushed a commit to reginaalyssa/plots2 that referenced this pull request Oct 16, 2021

Remove "parent" in tag.rb (publiclab#9404)

634c1df

* remove parent publiclab#9403 * remove "parent" * Fix test errors

billymoroney1 pushed a commit to billymoroney1/plots2 that referenced this pull request Dec 28, 2021

Remove "parent" in tag.rb (publiclab#9404)

45baa18

* remove parent publiclab#9403 * remove "parent" * Fix test errors

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove "parent" in tag.rb #9404

Remove "parent" in tag.rb #9404

jcads commented Mar 31, 2021 •

edited

Loading

gitpod-io bot commented Mar 31, 2021 •

edited

Loading

codecov bot commented Mar 31, 2021 •

edited

Loading

jcads commented Mar 31, 2021

jywarren left a comment

jywarren Apr 13, 2021

jcads Apr 14, 2021

jywarren Apr 13, 2021

jywarren Apr 13, 2021

jcads commented Apr 14, 2021

RuthNjeri commented Apr 15, 2021

codeclimate bot commented Apr 16, 2021

jywarren commented Apr 17, 2021

	def add_parent
	if logged_in_as(['admin'])
	@tag = Tag.find_by(name: params[:name])
	@tag.update_attribute('parent', params[:parent])
	if @tag.save
	flash[:notice] = "Tag parent added."
	else
	flash[:error] = "There was an error adding a tag parent."
	end
	redirect_to '/tag/' + @tag.name + '?_=' + Time.now.to_i.to_s
	else
	flash[:error] = "Only admins may add tag parents."
	end
	end

Remove "parent" in tag.rb #9404

Remove "parent" in tag.rb #9404

Conversation

jcads commented Mar 31, 2021 • edited Loading

gitpod-io bot commented Mar 31, 2021 • edited Loading

codecov bot commented Mar 31, 2021 • edited Loading

Codecov Report

jcads commented Mar 31, 2021

jywarren left a comment

Choose a reason for hiding this comment

jywarren Apr 13, 2021

Choose a reason for hiding this comment

jcads Apr 14, 2021

Choose a reason for hiding this comment

jywarren Apr 13, 2021

Choose a reason for hiding this comment

jywarren Apr 13, 2021

Choose a reason for hiding this comment

jcads commented Apr 14, 2021

RuthNjeri commented Apr 15, 2021

codeclimate bot commented Apr 16, 2021

jywarren commented Apr 17, 2021

jcads commented Mar 31, 2021 •

edited

Loading

gitpod-io bot commented Mar 31, 2021 •

edited

Loading

codecov bot commented Mar 31, 2021 •

edited

Loading