group.rb in ruby-jss-2.0.0b1

- old
+ new

@@ -110,21 +110,23 @@
     #
     def self.change_membership(group, add_members: [], remove_members: [], api: nil, cnx: Jamf.cnx)
       cnx = api if api
 
       raise Jamf::NoSuchItemError, "No #{self} matching '#{group}'" unless (group_id = valid_id group, cnx: cnx)
-      raise Jamf::UnsupportedError, "Not a static group, can't change membership directly" if map_all_ids_to(:is_smart, cnx: cnx)[group_id]
+      raise Jamf::UnsupportedError, "Not a static group, can't change membership" if map_all(:id, to: :is_smart, cnx: cnx)[group_id]
 
       add_members = [add_members].flatten
       remove_members = [remove_members].flatten
       return if add_members.empty? && remove_members.empty?
 
       # we must know the current group membership, because the API
       # will raise a conflict error if we try to remove a member
       # that isn't in the group (which is kinda lame - it should just
       # ignore this, like it does when we add a member that's already
       # in the group.)
+      # Its even more lame because we have to instantiate the group
+      # and part of the point of this class method is to avoid that.
       current_member_ids = fetch(id: group_id, cnx: cnx).member_ids
 
       # nil if no changes to be made
       xml_doc = change_membership_xml add_members, remove_members, current_member_ids
       return unless xml_doc
@@ -187,11 +189,10 @@
 
       member_removed ? removals : nil
     end
     private_class_method :member_removals_xml
 
-
     # Attributes
     #####################################
 
     # @return [Array<Hash>] the group membership
     #
@@ -205,38 +206,34 @@
     #
     attr_reader :members
 
     # @return [Boolean] is this a smart group
     attr_reader :is_smart
+    alias smart? is_smart
 
     # @return [Boolean] does this group send notifications when it changes?
     attr_reader :notify_on_change
+    alias notify_on_change? notify_on_change
+    alias notify? notify_on_change
 
-
     # Constructor
     #####################################
 
     # When creating a new group in the JSS, you must call .make with a :type key
     # and a value of :smart or :static, as well as a :name
     #
     # @see Jamf::APIObject
     #
     def initialize(**args)
-      if args[:id] == :new
-        raise Jamf::InvalidDataError, 'New group creation must specify a :type of :smart or :static' unless GROUP_TYPES.include? args[:type]
-      end
+      raise Jamf::InvalidDataError, 'New group creation must specify a :type of :smart or :static' if args[:id] == :new && !(GROUP_TYPES.include? args[:type])
 
       super
 
       @is_smart = @init_data[:is_smart] || (args[:type] == :smart)
 
       @members =
-        if @init_data[self.class::MEMBER_CLASS::RSRC_LIST_KEY]
-          @init_data[self.class::MEMBER_CLASS::RSRC_LIST_KEY]
-        else
-          []
-        end
+        @init_data[self.class::MEMBER_CLASS::RSRC_LIST_KEY] || []
     end # init
 
     # Public Instance Methods
     #####################################
 
@@ -249,13 +246,12 @@
     #   group to re-read the membership can happen too fast, the JSS won't know
     #   it exists yet and will throw a NoSuchItem error.  If that
     #   happens, try again this many times with a 1 second pause between attempts.
     #
     def create(calculate_members: true, retries: 10)
-      if @is_smart
-        raise Jamf::MissingDataError, 'No criteria specified for smart group' unless @criteria
-      end
+      raise Jamf::MissingDataError, 'No criteria specified for smart group' if @is_smart && !@criteria
+
       super()
 
       if calculate_members
         tries = 0
         while tries < retries
@@ -287,14 +283,14 @@
       params[:refresh] = true if params[:refresh].nil?
 
       if @in_jss
         raise Jamf::UnsupportedError, 'Updating this object in the JSS is currently not supported by ruby-jss' unless updatable?
 
-
         update refresh: params[:refresh]
       else
         raise Jamf::UnsupportedError, 'Creating this object in the JSS is currently not supported by ruby-jss' unless creatable?
+
         create calculate_members: params[:calculate_members], retries: params[:retries]
       end
     end
 
     # @see APIObject#delete
@@ -319,45 +315,60 @@
 
     # Change static group to smart group
     #
     # @param args[Hash] the options and settings use for switching the computer group from static group to smart group
     #
-    # @option args criteria[Array] The criterias to be user for the smart group
-    def set_smart(*params)
+    # @option args criteria[Array] The criteria to be user for the smart group
+    #
+    # @return [void]
+    def make_smart(**params)
       return if @is_smart
 
       params[:criteria] = [] if params[:criteria].nil?
 
       criteria = params[:criteria]
 
       @is_smart = true
       @need_to_update = true
     end
+    # backward compatility
+    alias set_smart make_smart
 
     # Change smart group to static group
     #
     # @param args[Hash] the options and settings use for switching the computer group from smart group to static group
     #
     # @option args preserve_members[Boolean] Should the smart group preserve it's current members?
-    def set_static(*params)
+    #
+    # @return [void]
+    def make_static(**params)
       return unless @is_smart
 
       preserve_members = params.include? :preserve_members
 
       @is_smart = false
 
-      clear() unless preserve_members
+      clear unless preserve_members
     end
-
+    # backward compatility
+    alias set_static make_static
+        
     # How many members of the group?
     #
     # @return [Integer] the number of members of the group
     #
     def size
       @members.count
     end
+    alias count size
 
+    # @return [Boolean] Is this a static group?
+    def static?
+      !smart?
+    end
+    alias is_static static?
+
     # @return [Array<String>] the names of the group members
     #
     def member_names
       @members.map { |m| m[:name] }
     end
@@ -379,19 +390,21 @@
     # @return [void]
     #
     def members=(new_members)
       raise UnsupportedError, "Smart group members can't be changed." if @is_smart
       raise InvalidDataError, 'Arg must be an array of names and/or ids' unless new_members.is_a? Array
+
       ok_members = []
       new_members.each do |m|
         ok_members << check_member(m)
       end
 
       ok_members.uniq!
 
       # make sure we've actually changed...
       return if members.map { |m| m[:id] }.sort == ok_members.map { |m| m[:id] }.sort
+
       @members = ok_members
       @need_to_update = true
     end
 
     # Add a member, by name or id
@@ -399,35 +412,46 @@
     # @param m[Integer,String] the id or name of the member to add
     #
     # @return [void]
     #
     def add_member(mem)
-      raise UnsupportedError, "Smart group members can't be changed." if @is_smart
+      raise UnsupportedError, "Smart group members can't be changed." if smart?
+
       @members << check_member(mem)
       @need_to_update = true
     end
 
     # Remove a member by id, or name
     #
-    # @param m[Integer,String] the id or name of the member to remove
+    # @param m[Integer,String] an identifier for the item to remove
     #
     # @return [void]
     #
     def remove_member(mem)
-      raise InvalidDataError, "Smart group members can't be changed." if @is_smart
-      raise InvalidDataError, "Can't remove nil" if mem.nil?
-      removed = @members.reject! { |mm| [mm[:id], mm[:name], mm[:username]].include? mem }
-      @need_to_update = true if removed
+      raise UnsupportedError, "Smart group members can't be changed." if smart?
+
+      # See if we have the identifier in the @members hash
+      id_to_remove = @members.select { |mm| mm.values.include? mem }.first&.dig :id
+      # But the members hash might not have SN, macaddr, etc, and never has udid, so
+      # look at the MEMBER_CLASS if needed
+      id_to_remove ||= self.class::MEMBER_CLASS.valid_id mem
+
+      # nothing to do if that id isn't one of our members
+      return unless id_to_remove && member_ids.include?(id_to_remove)
+
+      @members.delete_if { |k, v| k == :id && v == id_to_remove }
+      @need_to_update = true 
     end
 
     # Remove all members
     #
     # @return [void]
     #
     def clear
       raise InvalidDataError, "Smart group members can't be changed." if @is_smart
       return if @members.empty?
+
       @members.clear
       @need_to_update = true
     end
 
     # Immediatly add and/or remove members in this static group
@@ -456,18 +480,12 @@
     # @return [Array<Hash>] the refresh membership
     #
     def refresh_members
       @members = @cnx.c_get(@rest_rsrc)[self.class::RSRC_OBJECT_KEY][self.class::MEMBER_CLASS::RSRC_LIST_KEY]
     end
+  
 
-    # aliases
-
-    alias smart? is_smart
-    alias notify_on_change? notify_on_change
-    alias notify? notify_on_change
-    alias count size
-
     # Public Instance Methods
     #####################################
     private
 
     # Check that a potential group member is valid in the JSS.
@@ -475,16 +493,15 @@
     # An exception is raised if the device doesn't exist.
     #
     # @return [Hash{:id=>Integer,:name=>String}] the valid id and name
     #
     def check_member(m)
-      potential_members = self.class::MEMBER_CLASS.map_all_ids_to(:name, cnx: @cnx)
-      if m.to_s =~ /^\d+$/
-        return { id: m.to_i, name: potential_members[m] } if potential_members.key?(m.to_i)
-      else
-        return { name: m, id: potential_members.invert[m] } if potential_members.value?(m)
-      end
-      raise Jamf::NoSuchItemError, "No #{self.class::MEMBER_CLASS::RSRC_OBJECT_KEY} matching '#{m}' in the JSS."
+      desired_id = self.class::MEMBER_CLASS.valid_id m, cnx: @cnx
+      raise Jamf::NoSuchItemError, "No #{self.class::MEMBER_CLASS::RSRC_OBJECT_KEY} matching '#{m}' in the JSS." unless desired_id
+
+      desired_name = self.class::MEMBER_CLASS.map_all(:id, to: :name, cnx: @cnx)[desired_id]
+
+      { name: desired_name, id: desired_id }      
     end
 
     # the xml formated data for adding or updating this in the JSS,
     #
     def rest_xml