Skip to content

reduce list of projects for admin resolves #1187 #1238

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.

Sign up for GitHub

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

Open
MontrealSergiy wants to merge 11 commits into
base: dev
Choose a base branch
from
Open

reduce list of projects for admin resolves #1187 #1238

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
b0bc997
reduce list of projects for admin resolves #1187"
MontrealSergiy May 9, 2022
b5e7fc6
add other admin users (mostly for tools setup) fixes #1187
MontrealSergiy May 9, 2022
bc5dac7
add misc public categories reduce list of projects for admin resolve…
MontrealSergiy May 10, 2022
29bcd5d
add back few more projects for admin resolves #1187
MontrealSergiy May 11, 2022
7880426
refactor several pretty name methods into one, admin groups fixes #1187
MontrealSergiy May 30, 2022
816df7d
a refactoring of public group variable of a group table view resolves…
MontrealSergiy Jun 6, 2022
232d7b7
a refactoring of public group variable of a group table view resolves…
MontrealSergiy Jun 6, 2022
5941bee
Merge branch 'dev' into admingroups
MontrealSergiy Aug 31, 2022
809af83
minor fix in group select
MontrealSergiy Jan 23, 2023
69f24cb
add a redundant return to wg pretty categories
MontrealSergiy Apr 17, 2023
f35dd41
put back an old buggy pretty category optimization
MontrealSergiy Apr 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 21 additions & 3 deletions BrainPortal/app/helpers/select_box_helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,22 @@ def group_select(parameter_name = "group_id", options = {}, select_tag_options =
selected = selector.to_s
end

# for Admin filter out only public, invisible or non_assignable to reduce the clutter
if current_user.has_role? :admin_user
Copy link
Contributor

@natacha-beck natacha-beck May 13, 2022

Choose a reason for hiding this comment

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

When reading the Issue:

Those lists should be trimmed down to remove all private work projects

I think to keep the old groups and then remove only the private work projects will do what we want.
Instead of a long selection with a lot of conditions.

Copy link
Contributor Author

@MontrealSergiy MontrealSergiy May 14, 2022

Choose a reason for hiding this comment

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

that's was @prioux suggestion keep unassignable groups

Copy link
Contributor Author

@MontrealSergiy MontrealSergiy Jun 6, 2022

Choose a reason for hiding this comment

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

as a compromise maybe keep all the nonassignable group, rather than admin related

current_user_id = current_user.id
admin_ids = AdminUser.all.pluck(:id) || []
groups = groups.select do |g|
( g.is_a?(SystemGroup) || # everyone group, to make a private tool public
Copy link
Member

@prioux prioux Apr 20, 2023

Choose a reason for hiding this comment

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

  1. Reorder the conditions such that the fast conditions are checked first: g.public, g.invisible, etc
  2. When comparing with selected, the part selected.include(g.id.to_s) will wrongly select some group if for instance we selected is the string "12345" and the current group is ID "34". Use Array(selected).include... instead.

Copy link
Member

@prioux prioux Apr 20, 2023

Choose a reason for hiding this comment

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

Aren't there a better way to generate this list? Isn't this equivalent to

groups = SystemGroup.all.to_a | WorkGroup.where(:creator_id => admin_ids).to_a

(or close to that?)

g.is_a?(UserGroup) && (admin_ids & g.user_ids).present? || # admin usergroups
g.public ||
g.invisible ||
g.not_assignable && (g.user_ids & admin_ids ).present? || # notassingable admin groups
g.not_assignable && admin_ids.include?(g.creator_id) || # notassignable created by admin
(selected == g.id.to_s) || selected.include?(g.id.to_s) # already selected
)
end
end

# Optimize the labels for UserGroups and SiteGroups, by extracting in a hash
group_labels = {}
group_labels.merge!(UserGroup.prepare_pretty_labels(groups))
Expand Down Expand Up @@ -145,15 +161,17 @@ def group_select(parameter_name = "group_id", options = {}, select_tag_options =

# Step 1: My Work Projects first
ordered_category_grouped << [ "My Work Projects", category_grouped_pairs.delete("My Work Projects") ] if category_grouped_pairs["My Work Projects"]

ordered_category_grouped << [ "My Public Work Projects", category_grouped_pairs.delete("My Public Work Projects") ] if category_grouped_pairs["My Public Work Projects"]
# Step 2: All personal work projects first
category_grouped_pairs.keys.select { |proj| proj =~ /Personal Work Projects/ }.sort.each do |proj|
category_grouped_pairs.keys.select { |proj| proj =~ /Personal (Public )?Work Projects/ }.sort.each do |proj|
ordered_category_grouped << [ proj, category_grouped_pairs.delete(proj) ]
end

# Step 3: Other project categories, in that order
[ "Shared Work Projects", "Empty Work Projects", "Site Projects", "User Projects", "System Projects", "Invisible Projects", "Everyone Projects" ].each do |proj|
ordered_category_grouped << [ proj, category_grouped_pairs.delete(proj) ] if category_grouped_pairs[proj]
proj = proj.sub(" ", " Public ")
ordered_category_grouped << [ proj, category_grouped_pairs.delete(proj) ] if category_grouped_pairs[proj]

end

# Step 4: Other mysterious categories ?!?
Expand Down
12 changes: 11 additions & 1 deletion BrainPortal/app/models/work_group.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,17 @@ def self.prepare_pretty_category_names(groups = [], as_user = nil)
wgs
end

def pretty_category_name(as_user) #:nodoc:
def pretty_category_name(as_user = nil) #:nodoc:
category = pretty_category_name_no_pub(as_user)
if category.present? && self.public && ! category.include?('Public') && as_user.has_role?(:admin_user)
category.sub(" ", " Public ")
# category.split(' ', 2).insert(1, "Public").join # works even for empty str
else
category
end
end

def pretty_category_name_no_pub(as_user) #:nodoc:
Copy link
Contributor

@natacha-beck natacha-beck May 13, 2022

Choose a reason for hiding this comment

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

I do not really see the point to have an additional method here, I think we can only keep pretty_category_name and add extra logic in it.

Copy link
Contributor Author

@MontrealSergiy MontrealSergiy May 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I wanted to have something like Public Shared group, etc that is use 'public' a quantifier to existing group, In this way it is not just adding one Public Group category so adding a new method is justified imho.

BTW it also it's possible to use instead just Public group category then only small corrections to the existing method will be required (probably due to bug in the existing code)

Copy link
Contributor Author

@MontrealSergiy MontrealSergiy May 30, 2022

Choose a reason for hiding this comment

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

the additional method is removed

return @_pretty_category_name if @_pretty_category_name
if self.invisible?
@_pretty_category_name = 'Invisible Project'
Expand Down
2 changes: 1 addition & 1 deletion BrainPortal/app/views/groups/show.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@

<% if @group.is_a?(WorkGroup) %>
<div class="menu_bar">
<% if (@group.creator_id != current_user.id && !@group.public? && current_user.assignable_group_ids.include?(@group.id) %>
<% if @group.creator_id != current_user.id && !@group.public? && current_user.assignable_group_ids.include?(@group.id) %>
<%= link_to 'Leave Project', {:action => :unregister, :id => @group.id}, :class => "button", :method => :post %>
<% end %>
<% if @group.can_be_edited_by?(current_user) %>
Expand Down
52 changes: 43 additions & 9 deletions BrainPortal/app/views/shared/_group_tables.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,18 +24,29 @@

<%
if current_user.has_role? :admin_user
work_groups = WorkGroup.where(invisible: false).all
invis_groups = WorkGroup.where(invisible: true).all
admin_ids = AdminUser.all.pluck(:id) || []
public_groups = WorkGroup.where( invisible: false, public: true ).all
nonassign_groups = WorkGroup.joins(:users).where( invisible: false, public: false,
not_assignable: true, "users.id" => admin_ids ).all
nonassign_groups |= WorkGroup.where( invisible: false, public: false,
not_assignable: true, creator_id: admin_ids ).all
nonassign_groups = nonassign_groups.uniq
work_groups = model.groups.select { |g| g.is_a?(WorkGroup) && ! g.invisible && ! g.public &&
! nonassign_groups.include?(g) }
invis_groups = WorkGroup.where( invisible: true ).all
elsif current_user.has_role? :site_manager
work_groups = current_user.site.groups.where( :type => "WorkGroup", :invisible => false ).all | current_user.groups.where( :type => "WorkGroup", :invisible => false )
invis_groups = current_user.site.groups.where( :type => "WorkGroup", :invisible => true )
work_groups = current_user.site.groups.where( :type => "WorkGroup", :invisible => false ).all | current_user.groups.where( :type => "WorkGroup", :invisible => false )
invis_groups = current_user.site.groups.where( :type => "WorkGroup", :invisible => true )
public_groups = []
else
work_groups = []
invis_groups = []
work_groups = []
invis_groups = []
public_groups = []
end
work_groups = work_groups.sort { |a,b| a.name <=> b.name }
invis_groups = invis_groups.sort { |a,b| a.name <=> b.name }
class_name = model.class.sti_root_class.to_s.underscore
work_groups = work_groups.sort { |a,b| a.name <=> b.name }
invis_groups = invis_groups.sort { |a,b| a.name <=> b.name }
public_groups = public_groups.sort { |a,b| a.name <=> b.name }
class_name = model.class.sti_root_class.to_s.underscore
%>

<%
Expand Down Expand Up @@ -75,3 +86,26 @@

<% end %>



<% if public_groups.present? %>

<br>
<label>Public Projects</label>
<%= array_to_table(public_groups, :cols => 4, :td_class => 'left_align no_wrap') do |group,r,c| %>
<%= group_check_box.(group) %>
<% end %>

<% end %>



<% if nonassign_groups.present? %>

<br>
<label>Non-Assignable Projects</label>
<%= array_to_table(nonassign_groups, :cols => 4, :td_class => 'left_align no_wrap') do |group,r,c| %>
<%= group_check_box.(group) %>
<% end %>

<% end %>