Skip to content

Feature2#2

Open
Smokashi23 wants to merge 2 commits into
feature1from
feature2
Open

Feature2#2
Smokashi23 wants to merge 2 commits into
feature1from
feature2

Conversation

@Smokashi23

Copy link
Copy Markdown
Owner

No description provided.

Comment thread app/models/appt.rb
@@ -0,0 +1,3 @@
class Appt < ApplicationRecord

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Smokashi23 Use proper model name

Suggested change
class Appt < ApplicationRecord
class Appointment < ApplicationRecord

Comment thread app/models/role.rb
@@ -0,0 +1,4 @@
class Role < ApplicationRecord
has_many :users
validates :role, inclusion: { in: %w(doctor patient), message: "should be 'doctor' or 'patient'"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

message should be displayed through I18n.t

Comment thread config/database.yml
adapter: postgresql
pool: <%= ENV.fetch("RAILS_MAX_THREADS") { 5 } %>
timeout: 5000
username: postgres

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not push username password changes in database.yml

@Smokashi23 Smokashi23 Mar 4, 2024

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shubhh139 where to give credentials

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Smokashi23 either credentials.yml.enc file or .env file

class CreateRoles < ActiveRecord::Migration[7.1]
def change
create_table :roles do |t|
t.string :role_name

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
t.string :role_name
t.string :name

Inside Role table, name attribute will itself be self-explanatory

create_table :appts do |t|
t.references :user, foreign_key: true, null:false
t.references :slot, foreign_key: true, null:false

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary space detected

Comment thread test/models/slot_test.rb

class SlotTest < ActiveSupport::TestCase
# test "the truth" do
# assert true

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary file pushed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants