Feature/crud admin user #17
Conversation
LeandroSoria2
requested review from
DJA89,
JulianPasquale,
Ricoch,
AgostinaDev,
diego-suarez,
sebastiancaraballo and
lestairon
as code owners
LeandroSoria2
requested review from
diego-suarez,
AgostinaDev,
JulianPasquale and
sebastiancaraballo
and removed request for
diego-suarez,
JulianPasquale,
sebastiancaraballo,
AgostinaDev,
DJA89,
Ricoch and
lestairon
| def self.up | ||
| create_table :active_admin_comments do |t| | ||
| t.string :namespace | ||
| t.text :body | ||
| t.references :resource, polymorphic: true | ||
| t.references :author, polymorphic: true | ||
| t.timestamps | ||
| end | ||
| add_index :active_admin_comments, [:namespace] | ||
| end | ||
|
|
||
| def self.down | ||
| drop_table :active_admin_comments | ||
| end |
There was a problem hiding this comment.
Can we define the change method here?
There was a problem hiding this comment.
It was generated by active_admin, but I will change it.
There was a problem hiding this comment.
I'm not sure if active admin comments are necessary here, I think you can disable them in the configuration file
There was a problem hiding this comment.
If we are not going to use ActiveAdmin comments, then we need to remove this migrations as well
There was a problem hiding this comment.
I agree, this migration should be removed now that you've disabled comments in the configuration file @LeandroSoria2
There was a problem hiding this comment.
I changed in last commit
db/seeds.rb
Outdated
| @@ -5,3 +5,7 @@ | |||
| # | |||
| # movies = Movie.create([{ name: "Star Wars" }, { name: "Lord of the Rings" }]) | |||
| # Character.create(name: "Luke", movie: movies.first) | |||
| if Rails.env.development? | |||
There was a problem hiding this comment.
Seeds are not meant to be run in production usually. I think we can remove this condition
There was a problem hiding this comment.
I think it's better to keep this validation just in case someone accidentally runs this on production, wdyt?
There was a problem hiding this comment.
I don't think this can be done by accident actually. This is a task that needs to be manually run. Also seeds should be idempotent
| @@ -1,3 +1,2 @@ | |||
| class ApplicationController < ActionController::Base | |||
| include DeviseTokenAuth::Concerns::SetUserByToken | |||
There was a problem hiding this comment.
Why is this being removed? Don't we need to add it somewhere else?
There was a problem hiding this comment.
I have a problem with active admin and this is already in the API controller.
https://github.com/LeandroSoria2/TargetApi/blob/develop/app/controllers/api/v1/api_controller.rb
There was a problem hiding this comment.
@JulianPasquale It looks like including this line in the ApplicationController generates conflicts with active admin. He had another controller (ApiController) with this same DeviseTokenAuth inclusion already, which is the one he uses for endpoints, so he had to remove this line here in order to fix the issue he was having with active admin.
Here's a thread about the same issue Lean was having:
There was a problem hiding this comment.
ah so this is in the ApiController as well. If that's the case then it's ok to remove this 👍
What changes does this PR include?
Description
Add active admin gem and set up active admin and add sass rails gem
Summary
-Add to gem file active admin gem
-Add to gem file active admin gem
-Change application_controller
Trello
(https://trello.com/c/9narEXDz/30-3-as-an-admin-i-should-be-able-to-login-in-the-admin-page)