Remove usages of visible attribute from Page

This is a legacy attribute that was used to show the page in the navigation. Since the render_navigation helper has been removed and the feature has been replaced with a much better feature (Menus) we do not need this attribute anymore.

Also this attribute was used to generate the url of a page only taking visible parents into account. This also has been removed.

app/assets/stylesheets/alchemy/sitemap.scss

@@ -230,7 +230,7 @@ $sitemap-url-xlarge-width: 350px;
 
   .page_status {
     padding-left: 2 * $default-padding;
-    margin-right: 214px;
+    margin-right: 188px;
     margin-left: auto;
 
     @media screen and (min-width: $large-screen-break-point) {

app/controllers/alchemy/admin/pages_controller.rb

@@ -291,24 +291,14 @@ def create_tree(items, rootpage)
       # This function will add a node's own slug into their ancestor's path
       # in order to create the full URL of a node
       #
-      # NOTE: Invisible pages are not part of the full path of their children
-      #
       # @param [String]
       #   The node's ancestors path
       # @param [Hash]
       #   A children node
       #
       def process_url(ancestors_path, item)
         default_urlname = (ancestors_path.blank? ? "" : "#{ancestors_path}/") + item["slug"].to_s
-
-        pair = { my_urlname: default_urlname, children_path: default_urlname }
-
-        if item["visible"] == false
-          # children ignore an ancestor in their path if invisible
-          pair[:children_path] = ancestors_path
-        end
-
-        pair
+        { my_urlname: default_urlname, children_path: default_urlname }
       end
 
       def load_page

app/models/alchemy/page.rb

@@ -17,7 +17,6 @@
 #  rgt              :integer
 #  parent_id        :integer
 #  depth            :integer
-#  visible          :boolean          default(FALSE)
 #  locked_by        :integer
 #  restricted       :boolean          default(FALSE)
 #  robot_index      :boolean          default(TRUE)
@@ -44,7 +43,6 @@ class Page < BaseRecord
 
     DEFAULT_ATTRIBUTES_FOR_COPY = {
       autogenerate_elements: false,
-      visible: false,
       public_on: nil,
       public_until: nil,
       locked_at: nil,
@@ -78,7 +76,6 @@ class Page < BaseRecord
       :tag_list,
       :title,
       :urlname,
-      :visible,
       :layoutpage,
       :menu_id,
     ]

app/models/alchemy/page/fixed_attributes.rb

@@ -15,7 +15,6 @@ module Alchemy
   #       fixed_attributes:
   #         - public_on: nil
   #         - public_until: nil
-  #         - visible: false
   #
   class Page::FixedAttributes
     attr_reader :page

app/models/alchemy/page/page_naming.rb

@@ -22,7 +22,7 @@ module Page::PageNaming
         if: -> { title.blank? }
 
       after_update :update_descendants_urlnames,
-        if: :should_update_descendants_urlnames?
+        if: :saved_change_to_urlname?
 
       after_move :update_urlname!
     end
@@ -47,25 +47,21 @@ def slug
       urlname.to_s.split("/").last
     end
 
-    # Returns an array of visible/non-language_root ancestors.
-    def visible_ancestors
+    # Returns an array of non-language_root ancestors.
+    def non_root_ancestors
       return [] unless parent
 
       if new_record?
-        parent.visible_ancestors.tap do |base|
-          base.push(parent) if parent.visible?
+        parent.non_root_ancestors.tap do |base|
+          base.push(parent) unless parent.language_root?
         end
       else
-        ancestors.visible.contentpages.where(language_root: nil).to_a
+        ancestors.contentpages.where(language_root: nil).to_a
       end
     end
 
     private
 
-    def should_update_descendants_urlnames?
-      saved_change_to_urlname? || saved_change_to_visible?
-    end
-
     def update_descendants_urlnames
       reload
       descendants.each(&:update_urlname!)
@@ -100,13 +96,13 @@ def nested_url_name(value)
       (ancestor_slugs << convert_url_name(value)).join("/")
     end
 
-    # Slugs of all visible/non-language_root ancestors.
+    # Slugs of all non-language_root ancestors.
     # Returns [], if there is no parent, the parent is
     # the root page itself.
     def ancestor_slugs
       return [] if parent.nil?
 
-      visible_ancestors.map(&:slug).compact
+      non_root_ancestors.map(&:slug).compact
     end
   end
 end

app/models/alchemy/page/page_natures.rb

@@ -61,7 +61,6 @@ def locked?
     def status
       {
         public: public?,
-        visible: visible?,
         locked: locked?,
         restricted: restricted?,
       }

app/models/alchemy/page/page_scopes.rb

@@ -32,10 +32,6 @@ module Page::PageScopes
       #
       scope :not_locked, -> { where(locked_at: nil, locked_by: nil) }
 
-      # All visible pages
-      #
-      scope :visible, -> { where(visible: true) }
-
       # All not restricted pages
       #
       scope :not_restricted, -> { where(restricted: false) }

app/serializers/alchemy/page_tree_serializer.rb

@@ -53,7 +53,6 @@ def page_hash(page, has_children, level, folded)
         id: page.id,
         name: page.name,
         public: page.public?,
-        visible: page.visible?,
         restricted: page.restricted?,
         page_layout: page.page_layout,
         slug: page.slug,
@@ -105,7 +104,6 @@ def page_permissions(page, ability)
     def page_status_titles(page)
       {
         public: page.status_title(:public),
-        visible: page.status_title(:visible),
         restricted: page.status_title(:restricted),
       }
     end

app/views/alchemy/admin/pages/_form.html.erb

@@ -10,7 +10,6 @@
     <div class="control_group">
       <%= render 'alchemy/admin/pages/publication_fields' %>
       <%= page_status_checkbox(@page, :restricted) %>
-      <%= page_status_checkbox(@page, :visible) %>
       <% if configuration(:sitemap)['show_flag'] %>
         <%= page_status_checkbox(@page, :sitemap) %>
       <% end %>

app/views/alchemy/admin/pages/_page.html.erb

@@ -1,4 +1,4 @@
-<li id="page_{{id}}" class="page_level_{{level}} {{page_layout}}" data-slug="{{slug}}" data-restricted="{{restricted}}" data-visible="{{visible}}">
+<li id="page_{{id}}" class="page_level_{{level}} {{page_layout}}" data-slug="{{slug}}" data-restricted="{{restricted}}">
   <div class="sitemap_page{{#if locked}} locked{{/if}}" name="{{name}}">
     <div class="sitemap_left_images<% if @sorting %>{{#unless root}} handle{{/unless}}<% end %>">
       <% unless @sorting %>
@@ -150,10 +150,6 @@
         <i class="icon fas fa-fw fa-compass {{#unless public}}disabled{{/unless}}" data-fa-transform="shrink-2"></i>
         <span class="hint-bubble">{{status_titles.public}}</span>
       </span>
-      <span class="page_status with-hint">
-        <i class="icon fas fa-fw fa-eye {{#unless visible}}disabled{{/unless}}" data-fa-transform="shrink-2"></i>
-        <span class="hint-bubble">{{status_titles.visible}}</span>
-      </span>
       <span class="page_status with-hint">
         <i class="icon fas fa-fw fa-lock {{#unless restricted}}disabled{{/unless}}" data-fa-transform="shrink-2"></i>
         <span class="hint-bubble">{{status_titles.restricted}}</span>

app/views/alchemy/admin/pages/_page_infos.html.erb

@@ -2,10 +2,6 @@
   <%= render_icon(:compass, transform: 'shrink-2', class: @page.public? ? nil : 'disabled') %>
   <span class="hint-bubble"><%= page.status_title(:public) %></span>
 </span>
-<span class="page_status with-hint">
-  <%= render_icon(:eye, transform: 'shrink-2', class: @page.visible? ? nil : 'disabled') %>
-  <span class="hint-bubble"><%= page.status_title(:visible) %></span>
-</span>
 <span class="page_status with-hint">
   <%= render_icon(:lock, transform: 'shrink-2', class: @page.restricted? ? nil : 'disabled') %>
   <span class="hint-bubble"><%= page.status_title(:restricted) %></span>

app/views/alchemy/admin/pages/info.html.erb

@@ -24,10 +24,6 @@
         <%= render_icon(:compass, transform: 'shrink-2', class: @page.public? ? nil : 'disabled') %>
         <%= @page.status_title(:public) %>
       </span>
-      <span class="page_status">
-        <%= render_icon(:eye, transform: 'shrink-2', class: @page.visible? ? nil : 'disabled') %>
-        <%= @page.status_title(:visible) %>
-      </span>
       <span class="page_status">
         <%= render_icon(:lock, transform: 'shrink-2', class: @page.restricted? ? nil : 'disabled') %>
         <%= @page.status_title(:restricted) %>

config/alchemy/config.yml

@@ -11,7 +11,7 @@ auto_logout_time: 30
 
 # === Redirect Options
 #
-#   redirect_to_public_child  [Boolean]   # Alchemy redirects to the first public child page found, if a page is not visible.
+#   redirect_to_public_child  [Boolean]   # Alchemy redirects to the first public child page found, if a page is not public.
 #
 redirect_to_public_child: true
 

config/locales/alchemy.en.yml

@@ -481,9 +481,6 @@ en:
     page_published: "Published page"
     page_restricted: "restricted"
     page_states:
-      visible:
-        "true": "Page is visible in navigation."
-        "false": "Page is not visible in navigation."
       public:
         "true": "Page is published."
         "false": "Page is unpublished."
@@ -578,7 +575,6 @@ en:
         button_label: Upload image(s)
         upload_success: "Picture %{name} uploaded successfully"
         upload_failure: "Error while uploading %{name}: %{error}"
-    visible: "visible"
     want_to_create_new_language: "Do you want to create a new empty language tree?"
     want_to_make_copy_of_existing_language: "Do you want to copy an existing language tree?"
     "We need at least one default.": "A default language must exist."
@@ -826,7 +822,6 @@ en:
         updated_at: "Updated at"
         urlname: "URL-Path"
         slug: "Slug"
-        visible: "visible in navigation"
       alchemy/picture:
         image_file_name: "Filename"
         image_file_height: "Height"

lib/alchemy/permissions.rb

@@ -36,7 +36,7 @@ def initialize(user)
     module GuestUser
       def alchemy_guest_user_rules
         can([:show, :download], Alchemy::Attachment) { |a| !a.restricted? }
-        can :see,               Alchemy::Page,       restricted: false, visible: true
+        can :see, Alchemy::Page, restricted: false
 
         can :read, Alchemy::Content, Alchemy::Content.available.not_restricted do |c|
           c.public? && !c.restricted? && !c.trashed?
@@ -64,7 +64,7 @@ def alchemy_member_rules
 
         # Resources
         can [:show, :download], Alchemy::Attachment
-        can :see,               Alchemy::Page, restricted: true, visible: true
+        can :see, Alchemy::Page, restricted: true
 
         can :read, Alchemy::Content, Alchemy::Content.available do |c|
           c.public? && !c.trashed?

spec/controllers/alchemy/pages_controller_spec.rb

@@ -256,9 +256,9 @@ module Alchemy
       describe "url nesting" do
         render_views
 
-        let(:catalog) { create(:alchemy_page, :public, name: "Catalog", urlname: "catalog", parent: default_language_root, language: default_language, visible: true) }
-        let(:products) { create(:alchemy_page, :public, name: "Products", urlname: "products", parent: catalog, language: default_language, visible: true) }
-        let(:product) { create(:alchemy_page, :public, name: "Screwdriver", urlname: "screwdriver", parent: products, language: default_language, autogenerate_elements: true, visible: true) }
+        let(:catalog) { create(:alchemy_page, :public, name: "Catalog", urlname: "catalog", parent: default_language_root, language: default_language) }
+        let(:products) { create(:alchemy_page, :public, name: "Products", urlname: "products", parent: catalog, language: default_language) }
+        let(:product) { create(:alchemy_page, :public, name: "Screwdriver", urlname: "screwdriver", parent: products, language: default_language, autogenerate_elements: true) }
 
         before do
           allow(Alchemy.user_class).to receive(:admins).and_return(OpenStruct.new(count: 1))

spec/dummy/config/alchemy/page_layouts.yml

@@ -6,7 +6,6 @@
     page_layout: readonly
     public_on: ~
     public_until: ~
-    visible: false
     restricted: false
     name: false
     urlname: false

spec/features/admin/dashboard_spec.rb

@@ -10,7 +10,7 @@
   end
 
   describe "Locked pages summary" do
-    let(:a_page) { create(:alchemy_page, :public, visible: true) }
+    let(:a_page) { create(:alchemy_page, :public) }
 
     it "should initially show no pages are locked" do
       visit admin_dashboard_path

spec/features/admin/page_editing_feature_spec.rb

@@ -41,7 +41,7 @@
   end
 
   context "as admin" do
-    let(:a_page) { create(:alchemy_page, :public, visible: true) }
+    let(:a_page) { create(:alchemy_page, :public) }
 
     before { authorize_user(:as_admin) }
 

spec/features/page_feature_spec.rb

@@ -10,7 +10,7 @@
   end
 
   let(:public_page) do
-    create(:alchemy_page, :public, visible: true, name: "Page 1")
+    create(:alchemy_page, :public, name: "Page 1")
   end
 
   let(:public_child) do
@@ -139,12 +139,12 @@
   describe "navigation rendering" do
     context "with menu available" do
       let(:menu) { create(:alchemy_node, menu_type: "main_menu") }
-      let(:page1) { create(:alchemy_page, :public, visible: true, name: "Page 1") }
-      let(:page2) { create(:alchemy_page, :public, visible: true, name: "Page 2") }
+      let(:page1) { create(:alchemy_page, :public, name: "Page 1") }
+      let(:page2) { create(:alchemy_page, :public, name: "Page 2") }
       let!(:node1) { create(:alchemy_node, page: page1, parent: menu) }
       let!(:node2) { create(:alchemy_node, page: page2, parent: menu) }
 
-      it "should show the navigation with all visible pages" do
+      it "should show the navigation with all pages" do
         visit "/"
         within("nav ul") do
           expect(page).to have_selector('li a[href="/page-1"], li a[href="/page-2"]')

spec/features/page_redirects_spec.rb

@@ -10,7 +10,7 @@
   end
 
   let(:public_page) do
-    create(:alchemy_page, :public, visible: true, name: "Page 1")
+    create(:alchemy_page, :public, name: "Page 1")
   end
 
   let(:public_child) do
@@ -89,7 +89,6 @@
       before do
         public_page.update(
           public_on: nil,
-          visible: false,
           name: "Not Public",
           urlname: "",
         )
@@ -98,7 +97,7 @@
 
       it "redirects to public child" do
         visit "/not-public"
-        expect(page.current_path).to eq("/public-child")
+        expect(page.current_path).to eq("/not-public/public-child")
       end
 
       context "with only unpublished pages in page tree" do
@@ -117,7 +116,7 @@
         it "redirects to public child with prefixed locale" do
           allow(::I18n).to receive(:default_locale).and_return(:de)
           visit "/not-public"
-          expect(page.current_path).to eq("/en/public-child")
+          expect(page.current_path).to eq("/en/not-public/public-child")
         end
       end
     end
@@ -138,7 +137,6 @@
           before do
             default_language_root.update(
               public_on: nil,
-              visible: false,
               name: "Not Public",
               urlname: "",
             )
@@ -245,7 +243,6 @@
     context "redirects to public child" do
       before do
         public_page.update(
-          visible: false,
           public_on: nil,
           name: "Not Public",
           urlname: "",
@@ -255,12 +252,12 @@
 
       it "if requested page is unpublished" do
         visit "/not-public"
-        expect(page.current_path).to eq("/public-child")
+        expect(page.current_path).to eq("/not-public/public-child")
       end
 
       it "with normal url, if requested url has nested language code and is not public" do
         visit "/en/not-public"
-        expect(page.current_path).to eq("/public-child")
+        expect(page.current_path).to eq("/not-public/public-child")
       end
     end