feat(slack): set timezone from browser or country when provisioning slack users via SCIM

[System-End]

Prefer the user's browser-detected IANA timezone (stored on IdentitySession) when creating a Slack user via SCIM. If no browser timezone is available, fall back to a best-guess timezone for the user's country using TZInfo.

[Copilot]

Pull request overview

This PR enhances SCIM user provisioning for Slack by automatically setting the user's timezone based on their browser-detected IANA timezone from their most recent session, with a fallback to a country-based timezone lookup using TZInfo when session data is unavailable.

Changes:

• Added timezone_for_identity method to determine timezone from session or country
• Integrated timezone detection into build_user_payload to set timezone when provisioning Slack users
• Added error handling and logging for invalid timezone identifiers and country codes

Comments suppressed due to low confidence (4)

app/services/scim_service.rb:333

• The top-level rescue clause at line 331 will catch exceptions that have already been handled by the specific rescue blocks above (TZInfo::InvalidTimezoneIdentifier and TZInfo::InvalidCountryCode). Since both TZInfo-specific errors are already rescued and logged with more specific messages, this outer rescue is redundant and makes the error handling logic less clear. Consider removing it, or if there are other exceptions you want to catch, be more specific about which exceptions warrant this fallback.

    rescue => e
      Rails.logger.warn "Failed to determine timezone for identity #{identity.id}: #{e.message}"
      nil

app/services/scim_service.rb:334

• This new timezone detection logic lacks test coverage. Consider adding tests to verify: 1) that browser-detected timezones from sessions are correctly retrieved and validated, 2) that the country-based fallback works correctly when session timezone is not available, 3) that invalid timezone identifiers are handled gracefully, 4) that invalid country codes are handled gracefully, and 5) that the method returns nil when both approaches fail. Given the complexity and the potential for edge cases (invalid timezones, missing data, etc.), this functionality would benefit from comprehensive test coverage.

    def timezone_for_identity(identity)
      # Prefer browser-detected timezone from the user's most recent session
      session_tz = identity.sessions.order(created_at: :desc).pick(:timezone)
      if session_tz.present?
        begin
          TZInfo::Timezone.get(session_tz)
          return session_tz
        rescue TZInfo::InvalidTimezoneIdentifier
          Rails.logger.warn "Invalid session timezone '#{session_tz}' for identity #{identity.id}"
        end
      end

      # Fall back to country-based timezone
      country_code = identity.country
      if country_code.present?
        begin
          country = TZInfo::Country.get(country_code)
          zone_id = country.zone_identifiers.first
          return zone_id if zone_id.present?
        rescue TZInfo::InvalidCountryCode
          Rails.logger.warn "Invalid country code '#{country_code}' for timezone lookup"
        end
      end

      nil
    rescue => e
      Rails.logger.warn "Failed to determine timezone for identity #{identity.id}: #{e.message}"
      nil
    end

app/services/scim_service.rb:308

• The query retrieves the timezone from the most recent session by creation date, but this could include expired or signed-out sessions. Consider filtering to only active sessions (not expired and not signed out) to get a more accurate timezone. This would ensure the timezone reflects the user's current/recent active usage rather than potentially stale data from an old session.

      session_tz = identity.sessions.order(created_at: :desc).pick(:timezone)

app/services/scim_service.rb:323

• Using the first timezone from country.zone_identifiers may not be the best choice for countries with multiple timezones. For example, the US has multiple timezones, and picking the first one could result in an incorrect timezone for most users. Consider using a more sophisticated approach, such as using the most populous timezone or maintaining a mapping of preferred timezones per country.

          zone_id = country.zone_identifiers.first

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[System-End]

@24c02 any changrs needed?