-
Notifications
You must be signed in to change notification settings - Fork 0
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
Code review #16
base: code-review
Are you sure you want to change the base?
Code review #16
Conversation
Add reusable components
Url model and controller
Add shorten url functionality
Handle shorten link
Handle pin and unpin url
UI improvements
Error handling
Improve table and input ui
before_action :find_url, only: %i[show update] | ||
|
||
def index | ||
urls = Url.all.order("pinned DESC, updated_at DESC") |
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.
Create a scope in Url model and use it
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.
|
||
def create | ||
url = Url.new(url_params) | ||
url.slug = generate_slug |
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.
Controller's only job is to receive a request, fetch data from the db and send it to view.
All other business logic should go in the model, like generate_slug
@@ -1,4 +1,9 @@ | |||
Rails.application.routes.draw do | |||
|
|||
resources :urls, only: %i[index create] | |||
get '/:slug' => "urls#show" |
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.
Make routes resourceful always. In this example, use slugs/:slug instead or /:slug
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.
Good.
No description provided.