Merge pull request #1791 from codidact/0valt/1099/license-pref

Fix default license preference choice's option label

Author: Oaphi

app/assets/javascripts/preferences.js

@@ -1,4 +1,4 @@
-$(() => {
+document.addEventListener('DOMContentLoaded', () => {
   $('.js-user-pref').on('change', async (ev) => {
     const $tgt = $(ev.target);
     let value;
@@ -10,6 +10,7 @@ $(() => {
     }
     const prefName = $tgt.attr('data-pref');
     const community = $tgt.attr('data-community') === 'true';
+
     await QPixel.setPreference(prefName, value, community);
   });
 
@@ -25,4 +26,4 @@ $(() => {
       $(e).addClass('is-yellow');
     }
   });
-});
+});

app/assets/javascripts/qpixel_api.js

@@ -238,13 +238,9 @@ window.QPixel = {
 
     const data = await resp.json();
 
-    if (data.status !== 'success') {
-      console.error(`Preference persist failed (${name})`);
-      console.error(resp);
-    }
-    else {
+    QPixel.handleJSONResponse(data, (data) => {
       QPixel._updatePreferencesLocally(data.preferences);
-    }
+    });
   },
 
   filters: async () => {

app/controllers/users_controller.rb

@@ -193,12 +193,14 @@ def set_preference
       community_key = "prefs.#{current_user.id}.community.#{RequestContext.community_id}"
       key = params[:community].present? && params[:community] ? community_key : global_key
       current_user.validate_prefs!
-      render json: { status: 'success', success: true,
+      render json: { status: 'success',
                      count: RequestContext.redis.hset(key, params[:name], params[:value].to_s),
                      preferences: current_user.preferences }
     else
-      render json: { status: 'failed', success: false, errors: ['Both name and value parameters are required'] },
-             status: 400
+      render json: { status: 'failed',
+                     message: 'Failed to save the preference',
+                     errors: ['Both name and value parameters are required'] },
+             status: :bad_request
     end
   end
 

app/views/posts/_form.html.erb

@@ -1,15 +1,16 @@
 <%#
-  Post create/edit form covering all post types.
-  Variables:
-    post          : a Post instance to work on (either via .new or .find - doesn't matter if it has content or not)
-    submit_path   : the URL to submit the form to (must accept POST requests)
-    post_type     : a PostType instance describing the requested post type
-    category      : a Category instance, required if the post type has a category, indicating which category to post in
-    parent        : a Post instance containing the parent for this post (required if the post type has a parent)
-    inline_parent : optional, default  true: display an inline copy of the parent? (if present)
-    type_summary  : optional, default  true: display a summary of the requested post type?
-    edit_comment  : optional, default false: include a field for an edit comment?
-%>
+   "Post create/edit form covering all post types.
+
+    Variables:
+      post          : a Post instance to work on (either via .new or .find - doesn't matter if it has content or not)
+      submit_path   : the URL to submit the form to (must accept POST requests)
+      post_type     : a PostType instance describing the requested post type
+      category      : a Category instance, required if the post type has a category, indicating which category to post in
+      parent        : a Post instance containing the parent for this post (required if the post type has a parent)
+      inline_parent : optional, default  true: display an inline copy of the parent? (if present)
+      type_summary  : optional, default  true: display a summary of the requested post type?
+      edit_comment  : optional, default false: include a field for an edit comment?
+"%>
 
 <%
   # defaults
@@ -54,8 +55,14 @@
     </p>
   <% end %>
 
-  <%= render 'shared/body_field', f: f, field_name: :body_markdown, field_label: t('posts.body_label'), post: post,
-            cur_length: post.body_markdown&.length, min_length: min_body_length(category), max_length: max_body_length(category) %>
+  <%= render 'shared/body_field',
+             f: f,
+             field_name: :body_markdown,
+             field_label: t('posts.body_label'),
+             post: post,
+             cur_length: post.body_markdown&.length,
+             min_length: min_body_length(category),
+             max_length: max_body_length(category) %>
 
   <div class="rejected-elements notice is-warning hide">
     <h3>Unsupported HTML detected</h3>
@@ -144,15 +151,19 @@
         <% category_default = category.license %>
         <% user_default = user_preference('default_license', community: true) %>
         <% if site_default.present? %>
-          community default: <a href="javascript:void(0)" class="js-license-autofill" data-license-id="<%= site_default.id %>">
+          community default: <a href="javascript:void(0)"
+                                class="js-license-autofill"
+                                data-license-id="<%= site_default.id %>">
             <%= site_default.name %>
           </a>
         <% end %>
         <% if site_default.present? && category_default.present? %>
           &middot;
         <% end %>
         <% if category_default.present? %>
-          category default: <a href="javascript:void(0)" class="js-license-autofill" data-license-id="<%= category_default.id %>">
+          category default: <a href="javascript:void(0)"
+                               class="js-license-autofill"
+                               data-license-id="<%= category_default.id %>">
             <%= category_default.name %>
           </a>
         <% end %>

app/views/users/_prefs.html.erb

@@ -11,8 +11,10 @@
         <input class="js-user-pref" type="checkbox" data-pref="<%= name %>" <%= 'checked' if value == 'true' %>
                data-community="<%= community %>" />
       <% when 'choice' %>
-        <%= select_tag nil, options_for_select(preference_choice(pref_config), selected: value),
-                       include_blank: true, class: 'js-user-pref form-element', data: { pref: name, community: community } %>
+        <%= select_tag nil,
+                       options_for_select(preference_choice(pref_config), selected: value),
+                       class: 'js-user-pref form-element',
+                       data: { pref: name, community: community } %>
       <% when 'integer' %>
         <input class="js-user-pref form-element" type="number" data-pref="<%= name %>" value="<%= value %>"
                data-community="<%= community %>" />
@@ -22,4 +24,4 @@
       <% end %>
     </div>
   </div>
-<% end %>
+<% end %>

config/config/preferences.yml

@@ -38,6 +38,8 @@ default_license:
     Your choice of default license. This will override site and category default licensing.
   default: ~
   choice:
+    - name: Site or category default
+      value: ~
     - name: CC BY-SA 4.0
       value: CC BY-SA 4.0
     - name: CC BY 4.0

test/controllers/users_controller_test.rb

@@ -507,6 +507,43 @@ class UsersControllerTest < ActionController::TestCase
     assert(items.any? { |x| x.instance_of?(Flag) && x.id == declined_flag.id })
   end
 
+  test 'set_preference should correclty save valid preferences' do
+    sign_in users(:standard_user)
+
+    pref_key = 'default_license'
+    license = licenses(:cc_by_sa)
+
+    [nil, communities(:sample)].each do |community|
+      try_save_preference(pref_key, license, community: community)
+
+      assert_response(:success)
+      assert_valid_json_response
+
+      parsed_body = JSON.parse(response.body)
+      assert_equal 'success', parsed_body['status']
+      assert_not_nil parsed_body['preferences']
+
+      comm_key = community.present? ? 'community' : 'global'
+      assert_not_nil parsed_body['preferences'][comm_key]
+      assert_equal license.id.to_s, parsed_body['preferences'][comm_key][pref_key]
+    end
+  end
+
+  test 'set_preference should correctly handle invalid preferences' do
+    sign_in users(:standard_user)
+
+    [nil, communities(:sample)].each do |community|
+      post :set_preference, params: { community: community, format: :json }
+
+      assert_response(:bad_request)
+      assert_valid_json_response
+
+      parsed_body = JSON.parse(response.body)
+      assert_equal 'failed', parsed_body['status']
+      assert_not_nil parsed_body['message']
+    end
+  end
+
   private
 
   def create_other_user
@@ -516,4 +553,13 @@ def create_other_user
     other_user.community_users.create!(community: other_community)
     other_user
   end
+
+  def try_save_preference(name, value, community: nil)
+    post :set_preference, params: {
+      community: community,
+      name: name,
+      value: value,
+      format: :json
+    }
+  end
 end