Adding date_format_pref field for new user#247
Adding date_format_pref field for new user#247achitti2812 wants to merge 1 commit intoexpertiza:mainfrom
Conversation
Generated by 🚫 Danger |
|
🚨 RSpec Tests Report |
| rescue_from ActiveRecord::RecordNotFound, with: :institution_not_found | ||
| def action_allowed? | ||
| has_role?('Instructor') | ||
| has_role?('Instructor') || has_role?('Administrator') |
There was a problem hiding this comment.
Instead of using the disjunction in a lot of action_allowed methods, I believe there is a has_instructor_privileges? method that implements the disjunction.
| # Role IDs | ||
| STUDENT_ID = 1 | ||
| TEACHING_ASSISTANT_ID = 2 | ||
| # Role IDs (must match db/seeds.rb) |
There was a problem hiding this comment.
Should there be a db table with these correspondences?
| super(options.merge({ only: %i[id name parent_id] })) | ||
| end | ||
| end | ||
| end No newline at end of file |
| STUDENT_ID = 1 | ||
| TEACHING_ASSISTANT_ID = 2 | ||
| # Role IDs (must match db/seeds.rb) | ||
| SUPER_ADMINISTRATOR_ID = 1 |
There was a problem hiding this comment.
Could this just be SUPER_ADMINISTRATOR, etc.? All files would need to be refactored to effect this change, but it would enhance readability. But, would it lead to namespace conflicts? If it doesn't cause name conflicts, it would be worth doing.
| @@ -34,17 +34,17 @@ class User < ApplicationRecord | |||
| delegate :super_administrator?, to: :role | |||
|
|
|||
| def self.instantiate(record) | |||
There was a problem hiding this comment.
Please refactor, according to the recommendations in https://chatgpt.com/share/694abed0-f590-800d-ac2a-4d87911c5de3.
2 participants
No description provided.