Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

show all available watchers in json generator #32

Merged
merged 3 commits into from
Jul 21, 2022
Merged

Conversation

maimai77
Copy link

@maimai77 maimai77 commented Jul 8, 2022

IssuesHelper#users_for_new_issue_watchers returns empty array when there are more than 20 project members.

@maimai77 maimai77 requested a review from nishidayuya July 8, 2022 08:00
Copy link

@nishidayuya nishidayuya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🎉

@sanak
Copy link

sanak commented Jul 9, 2022

Hi @maimai77 @nishidayuya,

I tried this PR branch which relates with #31, but the change seems to drop Groups from the Watcher list.
(I checked it on Private child of eCookbook after loading Redmine test data by bundle exec rake db:fixtures:load.)

  • master branch:
    • master-private-child-watchers
  • available-watchers branch: => Last Group A Team :10 is missing.
    • pr32-private-child-watchers

I felt that the following comment's Suggestion-A way seems to be better,
#31 (comment)

so I tried it with the following changes and it seems to work as I expected at least the issue template setting view.

--- a/app/controllers/concerns/issue_templates_common.rb
+++ b/app/controllers/concerns/issue_templates_common.rb
@@ -109,7 +109,7 @@ module IssueTemplatesCommon
 
       if field == 'watcher_user_ids' && project_id.present?
         issue = Issue.new(tracker_id: tracker_id, project_id: project_id)
-        watchers = helpers.users_for_new_issue_watchers(issue)
+        watchers = users_for_new_issue_watchers_with_limit(issue, nil)
         value[:field_format] = 'list'
 
         value[:possible_values] = watchers.map { |user| "#{user.name} :#{user.id}" }
@@ -149,4 +149,21 @@ module IssueTemplatesCommon
     logger&.info "core_fields_map_by_tracker_id failed due to this error: #{e.message}"
     {}
   end
+
+  # https://github.com/redmine/redmine/blob/master/app/helpers/issues_helper.rb
+  # Returns an array of users that are proposed as watchers
+  # on the new issue form
+  def users_for_new_issue_watchers_with_limit(issue, limit = 21)
+    users = issue.watcher_users.select{|u| u.status == User::STATUS_ACTIVE}
+    if limit.present?
+      assignable_watchers = issue.project.principals.assignable_watchers.limit(limit)
+      if assignable_watchers.size < limit
+        users += assignable_watchers.sort
+      end
+    else
+      assignable_watchers = issue.project.principals.assignable_watchers
+      users += assignable_watchers.sort
+    end
+    users.uniq
+  end
 end

By the way, even if applying above changes, I couldn't set default watchers on a new issue view when more than 21 members, because Redmine seems to cut watcher checkbox list and show only Search for watchers to add link.
new-issue-watchers-list1

In this (>= 21 members) case, clicking Search for watchers to add link, then selecting all members from the setting, then clicking Add button steps are necessary, but I couldn't achieve it yet by code...
new-issue-watchers-list2

Here is my trial code, so if someone know how to achieve it, sharing the information is helpful... 🙇

--- a/assets/javascripts/issue_templates.js
+++ b/assets/javascripts/issue_templates.js
@@ -423,12 +423,27 @@ ISSUE_TEMPLATE.prototype = {
     document.getElementById('issue_template').dispatchEvent(changeEvent)
   },
   checkSelectedWatchers: function (values) {
-    let targets = document.querySelectorAll('input[name="issue[watcher_user_ids][]"]')
-    for (let i = 0; i < targets.length; i++) {
-      let target = targets[i]
-      if (values.includes(target.value)) {
-        target.checked = true
+    let targets = document.querySelectorAll('input[type="checkbox"][name="issue[watcher_user_ids][]"]')
+    if (targets.length > 0) {
+      for (let i = 0; i < targets.length; i++) {
+        let target = targets[i]
+        if (values.includes(target.value)) {
+          target.checked = true
+        }
+      }
+    } else {
+      // Click "Search for watchers to add" link to open "Add watchers" dialog
+      document.querySelector('span.search_for_watchers a').click()
+      // Check watchers from template setting
+      let searchTargets = document.querySelectorAll('div#users_for_watcher input[name="watcher[user_ids][]"]')
+      for (let i = 0; i < searchTargets.length; i++) {
+        let target = searchTargets[i]
+        if (values.includes(target.value)) {
+          target.checked = true
+        }
       }
+      // Click "Add" button to apply checked watchers and close the dialog
+      document.querySelector('form#new-watcher-form input[type="submit"]').click()
     }
   },

@sanak
Copy link

sanak commented Jul 9, 2022

Here is my trial code, so if someone know how to achieve it, sharing the information is helpful... 🙇

Well, I solve this issue with using the following MutationObserver way.
https://stackoverflow.com/questions/5525071/how-to-wait-until-an-element-exists/16726669#16726669

Also, I created the following PR #33 which covers above my comments.

Thanks,

@kyamada23 kyamada23 merged commit 80c9ebf into master Jul 21, 2022
@kyamada23 kyamada23 deleted the available-watchers branch July 21, 2022 00:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants