-
Notifications
You must be signed in to change notification settings - Fork 0
Make Room a model #546
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
base: master
Are you sure you want to change the base?
Make Room a model #546
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| class Room < ApplicationRecord | ||
| has_one :user, foreign_key: :room, primary_key: :number, dependent: :restrict_with_error, inverse_of: :room_record | ||
|
|
||
| validates :number, presence: true, uniqueness: true, length: { maximum: 6 }, | ||
| format: { with: /\A[A-Z0-9]+\z/, message: 'must be uppercase alphanumeric' } | ||
| validates :group, presence: true, length: { maximum: 6 }, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe a comment/example to understand what a "group" is?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Before I write it in the code, what do you think about:
|
||
| format: { with: /\A[A-Z0-9]+\z/, message: 'must be uppercase alphanumeric' } | ||
| validates :building, presence: true, inclusion: { in: ('A'..'F').to_a } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is "Foyer" in the F building? Accueil or Halle projet rooms in A?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Answered by the seed |
||
| validates :floor, presence: true, inclusion: { in: 0..3 } | ||
|
|
||
| # Returns rooms available for assignment: unoccupied rooms + the room already assigned to the given user | ||
| scope :available_for, lambda { |user| | ||
| occupied = User.where.not(id: user.id).where.not(room: nil).select(:room) | ||
| where.not(number: occupied) | ||
| } | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,8 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| class User < ApplicationRecord | ||
| belongs_to :room_record, class_name: 'Room', foreign_key: :room, primary_key: :number, inverse_of: :user, | ||
| optional: true | ||
| has_many :machines, dependent: :destroy | ||
| has_many :free_accesses, dependent: :destroy | ||
| has_many :free_accesses_by_date, lambda { | ||
|
|
@@ -15,7 +17,7 @@ class User < ApplicationRecord | |
| }, through: :sales_as_client, dependent: :destroy, class_name: 'Subscription', source: :subscription | ||
|
|
||
| normalizes :email, with: ->(email) { email.strip.downcase } | ||
| normalizes :room, with: ->(room) { room&.downcase&.upcase_first } | ||
| normalizes :room, with: ->(room) { room&.upcase&.presence } | ||
|
|
||
| # Since the Radius MD4 hash is broken anyway (see: https://kanidm.github.io/kanidm/master/integrations/radius.html#cleartext-credential-storage) | ||
| # we choose to store the wifi_password encrypted using Rails built-in encryption. | ||
|
|
@@ -27,13 +29,13 @@ class User < ApplicationRecord | |
| validates :lastname, presence: true, allow_blank: false | ||
| VALID_EMAIL_REGEX = /\A[\w+\-.]+@[a-z\d\-]+(\.[a-z\d\-]+)*\.[a-z]+\z/ | ||
| validates :email, presence: true, format: { with: VALID_EMAIL_REGEX }, uniqueness: true | ||
| # TODO: Make room regex case-sensitive once we fix support for 'DF1' with uppercase | ||
| VALID_ROOM_REGEX = /\A([A-F][0-3][0-9]{2}[a-b]?|DF[1-4])\z/i | ||
| validates :room, format: { with: VALID_ROOM_REGEX }, uniqueness: true, allow_nil: true | ||
| validates :room, uniqueness: true, allow_nil: true | ||
| validate :room_must_exist, if: -> { room.present? } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using the primary key would also drop all this validation code |
||
| validates :wifi_password, presence: true, allow_blank: false | ||
| validates :username, presence: true, uniqueness: true, allow_blank: false | ||
|
|
||
| before_validation :ensure_has_wifi_password | ||
| after_save :sync_room_to_sso, if: :saved_change_to_room? | ||
|
|
||
| # @return [Array<String>] | ||
| attr_accessor :groups | ||
|
|
@@ -114,4 +116,12 @@ def ensure_has_wifi_password | |
|
|
||
| self.wifi_password = "#{SecureRandom.base58(6)}-#{SecureRandom.base58(6)}-#{SecureRandom.base58(6)}" | ||
| end | ||
|
|
||
| def room_must_exist | ||
| errors.add(:room, 'must reference an existing room') unless Room.exists?(number: room) | ||
| end | ||
|
|
||
| def sync_room_to_sso | ||
| SsoMetadataService.sync_room(self) | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| class SsoMetadataService | ||
| SSO_BASE_URL = 'https://sso.rezoleo.fr' | ||
|
|
||
| # @param user [User] | ||
| def self.sync_room(user) | ||
| new.sync_room(user) | ||
| end | ||
|
|
||
| # @param user [User] | ||
| def sync_room(user) | ||
| return if user.oidc_id.blank? | ||
|
|
||
| unless production? | ||
| Rails.logger.info("[SSO] Dry-run: would sync room '#{user.room}' for user #{user.oidc_id}") | ||
| return | ||
| end | ||
|
|
||
| push_room_metadata(user) | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def production? | ||
| Rails.env.production? | ||
| end | ||
|
|
||
| def push_room_metadata(user) | ||
| if user.room.present? | ||
| post_room_metadata(user) | ||
| else | ||
| delete_room_metadata(user) | ||
| end | ||
| end | ||
|
|
||
| def post_room_metadata(user) | ||
| uri = URI("#{SSO_BASE_URL}/v2/users/#{user.oidc_id}/metadata") | ||
| req = Net::HTTP::Post.new(uri) | ||
| req['Authorization'] = "Bearer #{access_token}" | ||
| req.content_type = 'application/json' | ||
| req.body = { metadata: [{ key: 'room', value: Base64.strict_encode64(user.room) }] }.to_json | ||
|
|
||
| res = Net::HTTP.start(uri.hostname, uri.port, use_ssl: true) { |http| http.request(req) } | ||
|
|
||
| unless res.is_a?(Net::HTTPSuccess) | ||
| Rails.logger.error("[SSO] Failed to set room for user #{user.oidc_id}: #{res.code} #{res.body}") | ||
| end | ||
| rescue StandardError => e | ||
| Rails.logger.error("[SSO] Error setting room for user #{user.oidc_id}: #{e.message}") | ||
| end | ||
|
|
||
| def delete_room_metadata(user) | ||
| uri = URI("#{SSO_BASE_URL}/v2/users/#{user.oidc_id}/metadata") | ||
| uri.query = URI.encode_www_form([['keys[]', 'room']]) | ||
| req = Net::HTTP::Delete.new(uri) | ||
| req['Authorization'] = "Bearer #{access_token}" | ||
|
|
||
| res = Net::HTTP.start(uri.hostname, uri.port, use_ssl: true) { |http| http.request(req) } | ||
|
|
||
| unless res.is_a?(Net::HTTPSuccess) | ||
| Rails.logger.error("[SSO] Failed to delete room for user #{user.oidc_id}: #{res.code} #{res.body}") | ||
| end | ||
| rescue StandardError => e | ||
| Rails.logger.error("[SSO] Error deleting room for user #{user.oidc_id}: #{e.message}") | ||
| end | ||
|
|
||
| def access_token | ||
| Rails.application.credentials.sso_lea5_pat! | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,8 +20,13 @@ | |
| </div> | ||
| <div> | ||
| <%= f.label :room %> | ||
| <%= f.text_field :room, placeholder: 'A123b or DF1', minlength: 3, maxlength: 5, | ||
| pattern: '([A-Fa-f][0-3][0-9]{2}[a-b]?|[Dd][Ff][1-4])' %> | ||
| <%= f.text_field :room, list: 'rooms-datalist', autocomplete: 'off', | ||
| placeholder: 'ex : A105A', value: f.object.room %> | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you need the |
||
| <datalist id="rooms-datalist"> | ||
| <% Room.available_for(f.object).order(:number).each do |room| %> | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This query should be hoisted to the controller and passed as a local variable. |
||
| <option value="<%= room.number %>"></option> | ||
| <% end %> | ||
| </datalist> | ||
| </div> | ||
| <div> | ||
| <%= f.submit yield(:button_text) %> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| class CreateRooms < ActiveRecord::Migration[7.2] | ||
| def change | ||
| create_table :rooms do |t| | ||
| t.string :number, limit: 6, null: false | ||
| t.string :group, limit: 6, null: false | ||
|
Comment on lines
+6
to
+7
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The 6 characters limit is arbitrary? (as arbitrary as the regex before)
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, to fit the longest number I thought of : |
||
| t.string :building, limit: 1, null: false | ||
| t.integer :floor, null: false | ||
|
|
||
| t.timestamps | ||
| end | ||
| add_index :rooms, :number, unique: true | ||
|
|
||
| # Nullify any user rooms that don't exist in the rooms table | ||
| reversible do |dir| | ||
| dir.up do | ||
| execute <<~SQL.squish | ||
| UPDATE users SET room = NULL | ||
| WHERE room IS NOT NULL | ||
| AND room NOT IN (SELECT number FROM rooms) | ||
| SQL | ||
| end | ||
| end | ||
|
|
||
| remove_index :users, :room, name: 'index_users_on_room' | ||
| add_index :users, :room, unique: true, where: 'room IS NOT NULL', name: 'index_users_on_room' | ||
| add_foreign_key :users, :rooms, column: :room, primary_key: :number | ||
| end | ||
| end | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still unsure about using a non-fixed column as a foreign key, all to avoid one join...
If we use the standard primary key, what do you think of reversing the has_one/belong_to association? Like we do with IPs, it would make finding free rooms easier.