Allow Organizations to Update Slug/Username (#1411)

* Close anchor tag typo

* Remove confusing message in profile

* Fix typo in spec

* Add old_slug and old_old_slug fields to org

* Test for check_for_slug_change method

* Add ability for orgs to change slug

* Update copy to match new org language

* Add redirect for organizations with old slugs

* Use less queries for handling org/user redirect

* Use consistent syntax for user creation

* Test for organization slug redirects

* Refactor user article redirect

app/controllers/stories_controller.rb

@@ -36,31 +36,29 @@ class StoriesController < ApplicationController
   private
 
   def redirect_to_changed_username_profile
-    if @user = User.find_by_old_username(params[:username].tr("@", "").downcase)
+    potential_username = params[:username].tr("@", "").downcase
+    user_or_org = User.find_by("old_username = ? OR old_old_username = ?", potential_username, potential_username) ||
+      Organization.find_by("old_slug = ? OR old_old_slug = ?", potential_username, potential_username)
+    if user_or_org.class.name == "User"
+      @user = user_or_org
       redirect_to @user.path
       return
-    end
-    if @user = User.find_by_old_old_username(params[:username].tr("@", "").downcase)
-      redirect_to @user.path
+    elsif user_or_org.class.name == "Organization"
+      @organization = user_or_org
+      redirect_to @organization.path
       return
+    else
+      not_found
     end
-    not_found
   end
 
   def handle_possible_redirect
-    if @user = User.find_by_old_username(params[:username].tr("@", "").downcase)
-      if @user.articles.find_by_slug(params[:slug])
-        redirect_to "/#{@user.username}/#{params[:slug]}"
-        return
-      end
-    end
-    if @user = User.find_by_old_old_username(params[:username].tr("@", "").downcase)
-      if @user.articles.find_by_slug(params[:slug])
-        redirect_to "/#{@user.username}/#{params[:slug]}"
-        return
-      end
-    end
-    if @organization = @article.organization
+    potential_username = params[:username].tr("@", "").downcase
+    @user = User.find_by("old_username = ? OR old_old_username = ?", potential_username, potential_username)
+    if @user&.articles&.find_by_slug(params[:slug])
+      redirect_to "/#{@user.username}/#{params[:slug]}"
+      return
+    elsif @organization = @article.organization
       redirect_to "/#{@organization.slug}/#{params[:slug]}"
       return
     end

app/models/organization.rb

@@ -42,6 +42,7 @@ class Organization < ApplicationRecord
   after_save  :bust_cache
   before_save :generate_secret
   before_validation :downcase_slug
+  before_validation :check_for_slug_change
   before_validation :evaluate_markdown
 
   validate :unique_slug_including_users
@@ -53,10 +54,26 @@ class Organization < ApplicationRecord
     slug
   end
 
+  def old_username
+    old_slug
+  end
+
+  def old_old_username
+    old_old_slug
+  end
+
   def website_url
     url
   end
 
+  def check_for_slug_change
+    if slug_changed?
+      self.old_old_slug = old_slug
+      self.old_slug = slug_was
+      articles.find_each { |a| a.update(path: a.path.gsub(slug_was, slug)) }
+    end
+  end
+
   def path
     "/#{slug}"
   end

app/views/users/_org_admin.html.erb

@@ -43,7 +43,7 @@
   <h3>Grow the team</h3>
   <h5>Invite teammates by sending them the secret and the following instructions:</h5>
   <ol style="margin-bottom: 30px">
-    <li>Sign up at <a href="https://<%= ApplicationConfig["APP_DOMAIN"] %>">https://<%= ApplicationConfig["APP_DOMAIN"] %>/a>
+    <li>Sign up at <a href="https://<%= ApplicationConfig["APP_DOMAIN"] %>">https://<%= ApplicationConfig["APP_DOMAIN"] %></a>
     <li>Navigate to <a href="https://<%= ApplicationConfig["APP_DOMAIN"] %>/settings/organization">https://<%= ApplicationConfig["APP_DOMAIN"] %>/settings/organization</a>
     <li>Paste the secret code below and click <b>Join Organization</b></li>
   </ol>
@@ -59,6 +59,11 @@
     <%= f.label :name %>
     <%= f.text_field :name, maxlength: 50 %>
   </div>
+  <div class="field">
+    <%=  f.label :username %>
+    <%= f.text_field :slug, maxlength: 18, minlength: 2 %>
+    <i>Your organization's URL is: https://dev.to/<%= @organization.slug %></i>
+  </div>
   <%= f.label :profile_image %>
   <div class="field">
     <% if @organization.profile_image_url.present? %>

app/views/users/_profile.html.erb

@@ -31,7 +31,6 @@
   <div class="field">
     <%= f.label :username %>
     <%= f.text_field :username, maxlength: 30 %>
-    Signing out and back in will update your Twitter/Github username.
   </div>
   <div class="field">
     <%= f.label :name %>

db/migrate/20181227192353_add_old_usernames_to_organizations.rb

@@ -0,0 +1,6 @@
+class AddOldUsernamesToOrganizations < ActiveRecord::Migration[5.1]
+  def change
+    add_column :organizations, :old_slug, :string
+    add_column :organizations, :old_old_slug, :string
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20181219215401) do
+ActiveRecord::Schema.define(version: 20181227192353) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 
@@ -491,6 +491,8 @@ ActiveRecord::Schema.define(version: 20181219215401) do
     t.string "location"
     t.string "name"
     t.string "nav_image"
+    t.string "old_old_slug"
+    t.string "old_slug"
     t.string "profile_image"
     t.text "proof"
     t.string "secret"

spec/models/organization_spec.rb

@@ -92,4 +92,38 @@ RSpec.describe Organization, type: :model do
       expect(organization).not_to be_valid
     end
   end
+
+  describe "#check_for_slug_change" do
+    def create_article_for_organization
+      user.update(organization_id: organization.id, org_admin: true)
+      create(:article, organization_id: organization.id, user_id: user.id)
+    end
+
+    it "properly updates the slug/username" do
+      random_new_slug = "slug_#{rand(10000)}"
+      organization.update(slug: random_new_slug)
+      expect(organization.slug).to eq random_new_slug
+    end
+
+    it "updates old_slug to original slug if slug was changed" do
+      original_slug = organization.slug
+      organization.update(slug: "slug_#{rand(10000)}")
+      expect(organization.old_slug).to eq original_slug
+    end
+
+    it "updates old_old_slug properly if slug was changed and there was an old_slug" do
+      original_slug = organization.slug
+      organization.update(slug: "something_else")
+      organization.update(slug: "another_slug")
+      expect(organization.old_old_slug).to eq original_slug
+    end
+
+    it "updates the paths of the organization's articles" do
+      create_article_for_organization
+      new_slug = "slug_#{rand(10000)}"
+      organization.update(slug: new_slug)
+      article = Article.find_by(organization_id: organization.id)
+      expect(article.path).to include new_slug
+    end
+  end
 end

spec/models/user_spec.rb

@@ -184,7 +184,7 @@ RSpec.describe User, type: :model do
       expect(user).not_to be_valid
     end
 
-    it "changes old_username if old_old_username properly if username changes" do
+    it "changes old_username and old_old_username properly if username changes" do
       old_username = user.username
       random_new_username = "username_#{rand(100000000)}"
       user.update(username: random_new_username)

spec/requests/stories_show_spec.rb

@@ -1,8 +1,8 @@
 require "rails_helper"
 
 RSpec.describe "StoriesShow", type: :request do
-  let(:user) { FactoryBot.create(:user) }
-  let(:article) { FactoryBot.create(:article, user_id: user.id) }
+  let(:user)         { create(:user) }
+  let(:article)      { create(:article, user_id: user.id) }
 
   describe "GET /:username/:slug" do
     it "renders to appropriate page" do
@@ -45,6 +45,25 @@ RSpec.describe "StoriesShow", type: :request do
       expect(response.body).to redirect_to("/#{user.username}/#{article.slug}")
     end
 
+    context "when organization is present" do
+      let(:organization) { create(:organization) }
+
+      it "redirects to the appropriate page if given an organization's old slug" do
+        original_slug = organization.slug
+        organization.update(slug: "somethingnew")
+        get "/#{original_slug}"
+        expect(response.body).to redirect_to organization.path
+      end
+
+      it "redirects to the appropriate page if given an organization's old old slug" do
+        original_slug = organization.slug
+        organization.update(slug: "somethingnew")
+        organization.update(slug: "anothernewslug")
+        get "/#{original_slug}"
+        expect(response.body).to redirect_to organization.path
+      end
+    end
+
     it "renders second and third users if present" do
       user2 = create(:user)
       user3 = create(:user)