Add migration alter cas_users to cas_people#2
Conversation
|
|
||
| <tbody> | ||
| <% @people.each do |person| %> | ||
| <tr id="person-<%= person.id %>"> |
| </div> | ||
| </div> | ||
|
|
||
| <%= link_to "Novo Usuário", new_site_person_path(@site) if current_person.admin? %> |
| devise_for :users, | ||
| class_name: "Cas::User", | ||
| devise_for :people, | ||
| class_name: "Cas::Person", |
There was a problem hiding this comment.
Creio que o melhor seja
devise_for :users,
class_name: "Cas::Person",
config/routes.rb
Outdated
| mount Shrine.presign_endpoint(:cache) => "/files/cache/presign" | ||
|
|
||
| authenticate :user, ->(u){ u.roles.include?('admin') } do | ||
| authenticate :person, ->(u){ u.roles.include?('admin') } do |
kurko
left a comment
There was a problem hiding this comment.
Ótimo PR. Deve ter dado bastante trabalho. Agora que ele está feito a gente começa a ver uns detalhes que não havíamos pensado antes, como o current_user. Pequenos ajustes faltando pra finalizar.
👏 👏 👏 👏
| def set_current_user | ||
| @current_user = current_user | ||
| def set_current_person | ||
| @current_person = current_person |
There was a problem hiding this comment.
Para o usuário logado, o ideal é que a gente trate ele como user, portanto current_user seria mais adequado. Além disso, é o que a maioria dos devs usa como nomenclatura, então vai evitar confusão.
| before_action :authenticate_user! | ||
| before_action :set_current_user | ||
| before_action :set_user_sites | ||
| before_action :authenticate_person! |
There was a problem hiding this comment.
Acho que mudando o route.rb, aqui fica authenticate_user!
| @@ -1,5 +1,5 @@ | |||
| class Cas::Api::FilesController < Cas::ApplicationController | |||
| skip_before_action :authenticate_user! | |||
| skip_before_action :authenticate_person! | |||
There was a problem hiding this comment.
Acho que mudando o route.rb, aqui fica authenticate_user!
| def index | ||
| @activities = Cas::Activity | ||
| .where(site_id: current_user.sites.map(&:id)) | ||
| .where(site_id: current_person.sites.map(&:id)) |
There was a problem hiding this comment.
Acho que mudando o route.rb, aqui fica current_user
| @@ -1,5 +1,5 @@ | |||
| class Cas::FileUploadsController < Cas::ApplicationController | |||
| skip_before_action :authenticate_user! | |||
| skip_before_action :authenticate_person! | |||
There was a problem hiding this comment.
Acho que mudando o route.rb, aqui fica authenticate_user!
|
|
||
| <% @sections.each do |section| %> | ||
| <% next unless Cas::SectionConfig.new(section).accessible_by_user?(current_user) %> | ||
| <% next unless Cas::SectionConfig.new(section).accessible_by_person?(current_person) %> |
| <%= csrf_meta_tags %> | ||
| </head> | ||
| <body class="<%= "site-#{@site.id}" if @current_user.present? && @site.present? %>"> | ||
| <body class="<%= "site-#{@site.id}" if @current_person.present? && @site.present? %>"> |
There was a problem hiding this comment.
current_user. Precisa revisar outros pontos aqui nesse arquivo também
| rename_table :cas_sites_users, :cas_people_sites | ||
| rename_column :cas_people_sites, :user_id, :person_id | ||
| add_index :cas_people_sites, [:site_id, :person_id], using: "btree" | ||
| add_index :cas_activities, :person_id, using: "btree" |
There was a problem hiding this comment.
btree já é o padrão, não precisa estipular aqui. Pode omitir o using
| @@ -0,0 +1,12 @@ | |||
| class AlterCasUsersToCasPeople < ActiveRecord::Migration[5.0] | |||
| def change | |||
spec/acceptance/activities_spec.rb
Outdated
|
|
||
| let!(:site1) { create(:site, name: "mysite1", slug: "mysite1") } | ||
| let!(:site2) { create(:site, name: "mysite2", slug: "mysite2") } | ||
| let!(:site3) { create(:site, name: "mysite3", slug: "mysite3") } |
There was a problem hiding this comment.
Um deles precisa ser mysite porque o fixtures/cas.yml tem um site chamado mysite. Olha acima,
let!(:site1) { create(:site, slug: "mysite1") }
let!(:site2) { create(:site, slug: "mysite") } # <====
let!(:site3) { create(:site, slug: "mysite3") }| belongs_to :author, class_name: "::Cas::User" | ||
| has_many :images, ->{ where(media_type: :image).order("cas_media_files.order ASC") }, class_name: "::Cas::MediaFile", as: :attachable, dependent: :destroy | ||
| belongs_to :author, class_name: "::Cas::Person" | ||
| has_many :images, ->{ where(media_type: :image).order("cas_media_files.order ASC") }, class_name: Cas::MediaFile, as: :attachable, dependent: :destroy |
|
|
||
| belongs_to :attachable, polymorphic: true | ||
| belongs_to :author, class_name: "::Cas::User" | ||
| belongs_to :author, class_name: "Cas::Person" |
| class User < ApplicationRecord | ||
| ROLES = %w[admin editor writer].freeze | ||
| class Person < ApplicationRecord | ||
| ROLES = %w[admin editor writer visitor].freeze |
| has_many :sites_users, class_name: '::Cas::SitesUser' | ||
| has_many :sites, through: :sites_users | ||
| has_many :files, class_name: 'Cas::MediaFile', as: :attachable | ||
| has_many :people_site, class_name: 'Cas::PeopleSite' |
| has_many :sections, dependent: :destroy | ||
| has_many :sites_users, class_name: '::Cas::SitesUser' | ||
| has_many :users, through: :sites_users | ||
| has_many :people_site, class_name: '::Cas::PeopleSite' |
Cas_users was changed to cas_people so that it would be given a more general name, and thus it would facilitate the insertion of people in the system, even if these people are not really a user of the system.