Merge pull request #2331 from glowfic-constellation/add/bookmark-spec…

…ific-privacy Add bookmark-specific privacy
glowfic-constellation · Dec 23, 2024 · 093d9d3 · 093d9d3
2 parents 1503704 + ff5cabc
commit 093d9d3
Show file tree

Hide file tree

Showing 17 changed files with 286 additions and 156 deletions.
diff --git a/app/assets/javascripts/bookmarks/rename.js b/app/assets/javascripts/bookmarks/rename.js
@@ -1,17 +1,22 @@
-const originalValues = {};
+const originalNameTextboxes = {};
+const originalPublicCheckboxes = {};
 
 $(document).ready(function() {
   const textFields = $(`.bookmark-name-text-field`);
   for (const textField of textFields) {
-    originalValues[textField.dataset.bookmarkId] = $(textField).val();
+    originalNameTextboxes[textField.dataset.bookmarkId] = $(textField).val();
+  }
+  const checkBoxes = $(`.bookmark-public-checkbox`);
+  for (const checkBox of checkBoxes) {
+    originalPublicCheckboxes[checkBox.dataset.bookmarkId] = $(checkBox).prop("checked");
   }
 
-  if (!$(".rename-bookmark").length) return;
+  if (!$(".edit-bookmark").length) return;
 
-  $(".rename-bookmark").click(function() {
+  $(".edit-bookmark").click(function() {
     /* Button to rename a bookmark */
     const bookmarkId = this.dataset.bookmarkId;
-    const editors = $(`.bookmark-name-editor`);
+    const editors = $(`.bookmark-editor`);
     for (const editor of editors) {
       // Hide all editors other than the one clicked
       const editorId = editor.dataset.bookmarkId;
@@ -25,25 +30,27 @@ $(document).ready(function() {
 
     // Toggle the one clicked
     $(`.bookmark-name[data-bookmark-id="${bookmarkId}"]`).toggle();
-    $(`.bookmark-name-editor[data-bookmark-id="${bookmarkId}"]`).toggle();
+    $(`.bookmark-editor[data-bookmark-id="${bookmarkId}"]`).toggle();
     return false;
   });
 
-  $(".save-bookmark-name").click(function() {
+  $(".save-bookmark").click(function() {
     $(".loading").show();
 
     const bookmarkId = this.dataset.bookmarkId;
     const newName = $(`.bookmark-name-text-field[data-bookmark-id="${bookmarkId}"]`).val();
+    const newPublic = $(`.bookmark-public-checkbox[data-bookmark-id="${bookmarkId}"]`).prop("checked");
 
     $.authenticatedAjax({
       url: '/api/v1/bookmarks/'+bookmarkId,
       type: 'PATCH',
-      data: {'name': newName},
+      data: {'name': newName, 'public': newPublic},
       success: function(data) {
-        originalValues[bookmarkId] = newName;
+        originalNameTextboxes[bookmarkId] = newName;
+        originalPublicCheckboxes[bookmarkId] = newPublic;
         $(".loading").hide();
         $(`.bookmark-name[data-bookmark-id="${bookmarkId}"] span`).first().html(nameFromData(data.name));
-        $(`.bookmark-name-editor[data-bookmark-id="${bookmarkId}"]`).hide();
+        $(`.bookmark-editor[data-bookmark-id="${bookmarkId}"]`).hide();
         $(`.bookmark-name[data-bookmark-id="${bookmarkId}"]`).show();
         $(`.saveconf[data-bookmark-id="${bookmarkId}"]`).show().delay(2000).fadeOut();
       },
@@ -56,10 +63,11 @@ $(document).ready(function() {
     return false;
   });
 
-  $(".discard-bookmark-name").click(function() {
+  $(".discard-bookmark-changes").click(function() {
     const bookmarkId = this.dataset.bookmarkId;
     if (confirm('Are you sure you wish to discard your changes?')) {
-      $(`.bookmark-name-text-field[data-bookmark-id="${bookmarkId}"]`).val(originalValues[bookmarkId]);
+      $(`.bookmark-name-text-field[data-bookmark-id="${bookmarkId}"]`).val(originalNameTextboxes[bookmarkId]);
+      $(`.bookmark-public-checkbox[data-bookmark-id="${bookmarkId}"]`).prop("checked", originalPublicCheckboxes[bookmarkId]);
     }
     return false;
   });

diff --git a/app/assets/stylesheets/replies.scss b/app/assets/stylesheets/replies.scss
@@ -562,3 +562,9 @@ div.post-subheader { width: 100%; }
   width: 100%;
   height: 50px;
 }
+
+/* Bookmark Edit Form */
+.bookmark-editor-fields { margin-right: 20px; }
+@media (max-width: 850px) {
+  .bookmark-editor-fields, .bookmark-editor-buttons { display: block; margin-right: 0; }
+}
diff --git a/app/controllers/api/v1/bookmarks_controller.rb b/app/controllers/api/v1/bookmarks_controller.rb
@@ -1,29 +1,26 @@
 # frozen_string_literal: true
 class Api::V1::BookmarksController < Api::ApiController
   before_action :login_required
-  before_action :find_bookmark, except: :create
+  before_action :bookmark_ownership_required, except: :create
 
   resource_description do
     description 'Viewing and modifying bookmarks'
   end
 
-  api :POST, '/bookmarks', 'Create a bookmark for the current user at a reply. If one already exists, update its name.'
+  api :POST, '/bookmarks', 'Create a bookmark for the current user at a reply. If one already exists, update it.'
   header 'Authorization', 'Authorization token for a user in the format "Authorization" : "Bearer [token]"', required: true
   param :reply_id, :number, required: true, desc: "Reply ID"
   param :name, String, required: false, allow_blank: true, desc: "New bookmark's name"
+  param :public, :boolean, required: false, allow_blank: true, desc: "New bookmark's public status"
   error 403, "Reply is not visible to the user"
   error 404, "Reply not found"
   error 422, "Invalid parameters provided"
   def create
     return unless (reply = find_object(Reply, param: :reply_id))
-    unless reply.post.visible_to?(current_user)
-      access_denied
-      return
-    end
+    access_denied and return unless reply.post.visible_to?(current_user)
 
-    bookmark = Bookmark.where(user: current_user, reply: reply, post: reply.post, type: "reply_bookmark").first_or_initialize
-    bookmark.assign_attributes(name: params[:name])
-    unless bookmark.save
+    bookmark = Bookmark.where(user: current_user, reply: reply, type: "reply_bookmark").first_or_initialize
+    unless bookmark.update(params.permit(:name, :public).merge(post_id: reply.post_id))
       error = { message: 'Bookmark could not be created.' }
       render json: { errors: [error] }, status: :unprocessable_entity
       return
@@ -32,20 +29,16 @@ def create
     render json: bookmark.as_json
   end
 
-  api :PATCH, '/bookmarks/:id', 'Update a single bookmark. Currently only supports renaming.'
+  api :PATCH, '/bookmarks/:id', 'Update a single bookmark.'
   header 'Authorization', 'Authorization token for a user in the format "Authorization" : "Bearer [token]"', required: true
   param :id, :number, required: true, desc: "Bookmark ID"
-  param :name, String, required: true, allow_blank: true, desc: "Bookmark's new name"
+  param :name, String, required: false, allow_blank: true, desc: "Bookmark's new name"
+  param :public, :boolean, required: false, allow_blank: true, desc: "Bookmark's new public status"
   error 403, "Bookmark is not visible to the user"
   error 404, "Bookmark not found"
   error 422, "Invalid parameters provided"
   def update
-    if @bookmark.user.id != current_user.try(:id)
-      access_denied
-      return
-    end
-
-    unless @bookmark.update(name: params[:name])
+    unless @bookmark.update(params.permit(:name, :public))
       error = { message: 'Bookmark could not be updated.' }
       render json: { errors: [error] }, status: :unprocessable_entity
       return
@@ -61,11 +54,6 @@ def update
   error 404, "Bookmark not found"
   error 422, "Invalid parameters provided"
   def destroy
-    if @bookmark.user.id != current_user.try(:id)
-      access_denied
-      return
-    end
-
     unless @bookmark.destroy
       error = { message: 'Bookmark could not be removed.' }
       render json: { errors: [error] }, status: :unprocessable_entity
@@ -77,8 +65,9 @@ def destroy
 
   private
 
-  def find_bookmark
+  def bookmark_ownership_required
     return unless (@bookmark = find_object(Bookmark))
-    access_denied unless @bookmark.visible_to?(current_user)
+    access_denied and return unless @bookmark.visible_to?(current_user)
+    access_denied unless @bookmark.user.id == current_user.try(:id)
   end
 end
diff --git a/app/controllers/api/v1/replies_controller.rb b/app/controllers/api/v1/replies_controller.rb
@@ -27,22 +27,18 @@ def index
   api :GET, '/replies/:id/bookmark', "Load a user's bookmark attached to a reply if it exists and is visible"
   param :id, :number, required: true, desc: "Reply ID"
   param :user_id, :number, required: true, desc: "User ID"
-  error 403, "Reply's post or user's bookmarks are not visible"
+  error 403, "Reply's post is not visible to the user"
   error 404, "Reply or user not found"
   error 422, "Invalid parameters provided"
   def bookmark
     return unless (reply = find_object(Reply))
     return unless (user = find_object(User, param: :user_id))
     access_denied and return unless reply.post.visible_to?(current_user)
-    bookmark_not_found and return unless user.public_bookmarks || user.id == current_user.try(:id)
 
     bookmark = reply.bookmarks.find_by(user_id: user.id, type: "reply_bookmark")
+    bookmark_not_found and return unless bookmark&.visible_to?(current_user)
 
-    if bookmark.present?
-      render json: bookmark.as_json
-    else
-      bookmark_not_found
-    end
+    render json: bookmark.as_json
   end
 
   private

diff --git a/app/controllers/api/v1/users_controller.rb b/app/controllers/api/v1/users_controller.rb
@@ -41,12 +41,9 @@ def posts
   param :id, :number, required: true, desc: "User ID"
   param :post_id, :number, required: false, desc: "Post ID"
   param :page, :number, required: false, desc: 'Page in results (25 per page)'
-  error 403, "Bookmarks are not visible to the user"
   error 404, "User not found"
   error 422, "Invalid parameters provided"
   def bookmarks
-    render json: { bookmarks: [] } and return unless @user.public_bookmarks || @user.id == current_user.try(:id)
-
     bookmarks = @user.bookmarks.visible_to(current_user)
     bookmarks = bookmarks.where(post_id: params[:post_id]) if params[:post_id].present?
 

diff --git a/app/controllers/bookmarks_controller.rb b/app/controllers/bookmarks_controller.rb
@@ -2,34 +2,34 @@
 require 'will_paginate/array'
 
 class BookmarksController < ApplicationController
-  before_action :login_required, except: [:search]
-  before_action :find_model, only: [:destroy]
+  before_action :login_required, except: :search
+  before_action :bookmark_ownership_required, only: :destroy
 
   def search
     @page_title = 'Search Bookmarks'
     use_javascript('search')
     use_javascript('bookmarks/rename')
     return unless params[:commit].present?
     return unless (@user = User.find_by_id(params[:user_id]))
-    unless @user.id == current_user.try(:id) || @user.public_bookmarks
+
+    @search_results = @user.bookmarked_replies.bookmark_visible_to(@user, current_user)
+    if @search_results.empty?
       # Return empty list when a user's bookmarks are private
-      @search_results = Reply.none.paginate(page: 1)
+      @search_results = @search_results.paginate(page: 1)
       return
     end
 
-    @search_results = @user.bookmarked_replies
     if params[:post_id].present?
       @posts = Post.where(id: params[:post_id])
       @search_results = @search_results.where(post_id: params[:post_id])
     end
 
     @search_results = @search_results
-      .visible_to(current_user)
       .joins(:post)
       .order('posts.subject, replies.created_at, posts.id')
       .joins(:user)
       .left_outer_joins(:character)
-      .select('replies.*, bookmarks.name as bookmark_name, bookmarks.id as bookmark_id, characters.name, ' \
+      .select('replies.*, bookmarks.id as bookmark_id, bookmarks.name as bookmark_name, bookmarks.public as bookmark_public, characters.name, ' \
               'characters.screenname, users.username, users.deleted as user_deleted')
       .paginate(page: page)
 
@@ -50,14 +50,9 @@ def create
       return redirect_to posts_path
     end
 
-    bookmark = Bookmark.where(reply_id: @reply.id, user_id: current_user.id, post_id: @reply.post.id,
-      type: 'reply_bookmark',).first_or_initialize
+    bookmark = Bookmark.where(reply_id: @reply.id, user_id: current_user.id, type: 'reply_bookmark').first_or_initialize
     if bookmark.new_record?
-      if params[:bookmark_name].present?
-        bookmark.update!(name: params[:bookmark_name])
-      else
-        bookmark.save!
-      end
+      bookmark.update!(params.permit(:name, :public).merge(post_id: @reply.post_id))
       flash[:success] = "Bookmark added."
     else
       flash[:error] = "Bookmark already exists."
@@ -67,12 +62,6 @@ def create
   end
 
   def destroy
-    @reply = @bookmark.reply
-    unless @bookmark.user.id == current_user.try(:id)
-      flash[:error] = "You do not have permission to remove this bookmark."
-      redirect_back fallback_location: reply_path(@reply, anchor: "reply-#{@reply.id}") and return
-    end
-
     begin
       @bookmark.destroy!
     rescue ActiveRecord::RecordNotDestroyed => e
@@ -86,11 +75,17 @@ def destroy
 
   private
 
-  def find_model
+  def bookmark_ownership_required
     @bookmark = Bookmark.find_by_id(params[:id])
-    return if @bookmark&.visible_to?(current_user)
+    unless @bookmark&.visible_to?(current_user)
+      flash[:error] = "Bookmark could not be found."
+      redirect_to posts_path and return
+    end
+
+    @reply = @bookmark.reply
+    return if @bookmark.user.id == current_user.try(:id)
 
-    flash[:error] = "Bookmark could not be found."
-    redirect_to posts_path and return
+    flash[:error] = "You do not have permission to perform this action."
+    redirect_back fallback_location: reply_path(@reply, anchor: "reply-#{@reply.id}")
   end
 end
diff --git a/app/models/bookmark.rb b/app/models/bookmark.rb
@@ -11,13 +11,13 @@ class Bookmark < ApplicationRecord
 
   scope :_visible_user, ->(user) { where(user_id: user&.id).joins(:user).or(where(user: { public_bookmarks: true })) }
   scope :_visible_post, ->(user) { where(post_id: Post.visible_to(user).select(:id)) }
-  scope :visible_to, ->(user) { _visible_user(user)._visible_post(user) }
+  scope :visible_to, ->(user) { _visible_user(user).or(where(public: true))._visible_post(user) }
 
   def visible_to?(other_user)
-    return false unless other_user
     return false unless post.visible_to?(other_user)
     return false unless reply
-    return true if other_user.id == user.id
+    return true if public
+    return true if other_user.try(:id) == user.id
     user.public_bookmarks
   end
 end
diff --git a/app/models/reply.rb b/app/models/reply.rb
@@ -30,6 +30,7 @@ class Reply < ApplicationRecord
   )
 
   scope :visible_to, ->(user) { where(post_id: Post.visible_to(user).select(:id)) }
+  scope :bookmark_visible_to, ->(bookmark_owner, viewing_user) { where(bookmarks: bookmark_owner.bookmarks.visible_to(viewing_user)) }
 
   def post_page(per=25)
     per_page = per > 0 ? per : post.replies.count

diff --git a/app/views/bookmarks/search.haml b/app/views/bookmarks/search.haml
@@ -53,15 +53,20 @@
                         = image_tag "icons/accept.png", title: 'Saved', class: 'vmid', alt: ''
                         Saved
                     - if user_is_owner
-                      .bookmark-name-editor.hidden{ data: { bookmark_id: reply.bookmark_id } }
-                        = text_field_tag nil, reply.bookmark_name, class: "bookmark-name-text-field", data: { bookmark_id: reply.bookmark_id }
-                        = submit_tag 'Save', class: 'save-bookmark-name button', data: { bookmark_id: reply.bookmark_id }
-                        = submit_tag 'Discard Changes', class: 'discard-bookmark-name button', data: { bookmark_id: reply.bookmark_id }
-                        .loading.float-right.margin-top-7.hidden
-                          = image_tag 'icons/loading.gif', title: 'Loading...', class: 'vmid', alt: 'Loading...'
-                        .saveerror.float-right.margin-top-7.hidden{ data: { bookmark_id: reply.bookmark_id } }
-                          = image_tag "icons/exclamation.png", title: 'Error', class: 'vmid', alt: ''
-                          Error, please try again
+                      .bookmark-editor.hidden{ data: { bookmark_id: reply.bookmark_id } }
+                        %span.bookmark-editor-fields
+                          = text_field_tag nil, reply.bookmark_name, placeholder: "Bookmark Name", class: "bookmark-name-text-field vmid", data: { bookmark_id: reply.bookmark_id }
+                          &nbsp;&nbsp;&nbsp;
+                          = check_box_tag "public_#{reply.bookmark_id}", true, reply.bookmark_public, class: "bookmark-public-checkbox vmid", data: { bookmark_id: reply.bookmark_id }
+                          = label_tag "public_#{reply.bookmark_id}", 'Public', class: "vmid"
+                        %span.bookmark-editor-buttons
+                          = submit_tag 'Save', class: 'save-bookmark button vmid', data: { bookmark_id: reply.bookmark_id }
+                          = submit_tag 'Discard Changes', class: 'discard-bookmark-changes button vmid', data: { bookmark_id: reply.bookmark_id }
+                          .loading.float-right.margin-top-7.hidden
+                            = image_tag 'icons/loading.gif', title: 'Loading...', class: 'vmid', alt: 'Loading...'
+                          .saveerror.float-right.margin-top-7.hidden{ data: { bookmark_id: reply.bookmark_id } }
+                            = image_tag "icons/exclamation.png", title: 'Error', class: 'vmid', alt: ''
+                            Error, please try again
                 %tr
                   %td.vtop{ class: cycle('even'.freeze, 'odd'.freeze) }
                     .post-info-box
@@ -79,12 +84,12 @@
                       = link_to reply_path(reply, anchor: "reply-#{reply.id}") do
                         = image_tag "icons/link.png".freeze, title: 'Permalink'.freeze, alt: 'Permalink'.freeze
                       - if user_is_owner
-                        = image_tag "icons/bookmark_pencil.png".freeze, title: 'Rename Bookmark', alt: 'Rename Bookmark', class: 'rename-bookmark pointer', data: { bookmark_id: reply.bookmark_id }
+                        = image_tag "icons/bookmark_pencil.png".freeze, title: 'Rename Bookmark', alt: 'Rename Bookmark', class: 'edit-bookmark pointer', data: { bookmark_id: reply.bookmark_id }
                       - if (bookmark = reply.bookmark_by(current_user))
                         = link_to bookmark_path(bookmark), method: :delete do
                           = image_tag "icons/bookmark_delete.png".freeze, title: 'Remove Bookmark'.freeze, alt: 'Remove Bookmark'.freeze
                       - else
-                        = link_to bookmarks_path(at_id: reply.id, bookmark_name: reply.bookmark_name), method: :post do
+                        = link_to bookmarks_path(at_id: reply.id, name: reply.bookmark_name), method: :post do
                           = image_tag "icons/bookmark.png".freeze, title: 'Copy Bookmark'.freeze, alt: 'Copy Bookmark'.freeze
                     .post-content
                       = sanitize_written_content(reply.content, reply.editor_mode)

diff --git a/app/views/users/edit.haml b/app/views/users/edit.haml
@@ -88,7 +88,7 @@
         %th.sub Bookmarks
         %td{class: cycle('even', 'odd')}
           = f.check_box :public_bookmarks, class: 'width-15 vmid'
-          = f.label :public_bookmarks, "Make bookmarks public", class: 'vmid'
+          = f.label :public_bookmarks, "Make all bookmarks public", class: 'vmid'
       %tr
         %th.sub Character Quick Switcher
         %td{class: cycle('even', 'odd')}