Skip to content
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

Fix all standardrb errors and warnings #168

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ jobs:
ruby-version: '3.2.3'
bundler-cache: true # runs 'bundle install' and caches installed gems automatically

- name: Set up database
run: bin/rails db:setup
- name: Create a test database
run: bin/rails db:create

- name: Run tests
run: bin/rails test
38 changes: 36 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

# Installation

How to run the application on your machine.

## With Docker

Build and start the application with `docker compose up`. Once the application has successfully started, you should be able to visit it at http://localhost:3000/
Expand Down Expand Up @@ -40,6 +42,38 @@ Access the app via `localhost:5000`

# Contributing

* Please create a new branch for each request
* Branch name should include issue number. For example: `branchname-23`
If you are not already a project contributor, you'll need to create a fork of the project before taking any of the following steps.

* Please create a new branch for each pull request.
* Branch name should include issue number. For example: `branchname-23`.
* Write tests.
* Write code to make the tests pass.
* Run the test suite and linter to make sure it is good code.
* Commit your changes to a branch, push to GitHub, and open a Pull Request!

## Development Tools

We use Minitest for testing. You can run the test suite like this

```console
$ bin/rails db:create # one time setup
$ bin/rails test

# in docker
$ docker compose run stocks bin/rails db:create
$ docker compose run stocks bin/rails test
Comment on lines +63 to +64
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good, useful

```

We use a Ruby linting and formatting gem called [Standard Ruby](https://github.com/standardrb/standard). You can run it like this:

```console
$ bin/standardrb # run the linter and emit errors or warnings
$ bin/standardrb --fix # automatically fix errors

# in docker
$ docker compose run stocks bin/standardrb
$ docker compose run stocks bin/standardrb --fix
```

Every Pull Request is checked by a GitHub Action which runs `bin/rails test` and `bin/standardrb`. If either check fails, the Pull Request will get a ❌ instead of the more pleasant ✅.

6 changes: 3 additions & 3 deletions app/controllers/admin/students_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,17 +47,17 @@ def update
private

def create_portfolio_transaction!
return unless fund_amount.present?
return if fund_amount.blank?

PortfolioTransaction.create!(
portfolio: requested_resource.portfolio,
amount: fund_amount,
transaction_type: 'deposit'
transaction_type: "deposit"
)
end

def fund_amount
@fund_amount ||= params['student']['add_fund_amount']
@fund_amount ||= params["student"]["add_fund_amount"]
end
end
end
19 changes: 10 additions & 9 deletions app/controllers/classrooms_controller.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
class ClassroomsController < ApplicationController
before_action :set_classroom, only: %i[ show edit update destroy ]
before_action :set_classroom, only: %i[show edit update destroy]
before_action :authenticate_user!

# GET /classrooms or /classrooms.json
Expand Down Expand Up @@ -59,13 +59,14 @@ def destroy
end

private
# Use callbacks to share common setup or constraints between actions.
def set_classroom
@classroom = Classroom.find(params[:id])
end

# Only allow a list of trusted parameters through.
def classroom_params
params.require(:classroom).permit(:name, :year_id, :school_id, :grade)
end
# Use callbacks to share common setup or constraints between actions.
def set_classroom
@classroom = Classroom.find(params[:id])
end

# Only allow a list of trusted parameters through.
def classroom_params
params.require(:classroom).permit(:name, :year_id, :school_id, :grade)
end
end
12 changes: 7 additions & 5 deletions app/controllers/schools_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,25 @@ def index
end

# GET /schools/1 or /schools/1.json
def show; end
def show
end
Comment on lines -11 to +12
Copy link
Collaborator

Choose a reason for hiding this comment

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

I like the single lines aesthetically, but I'm not going to fight the standardrb defaults over it

Copy link
Member Author

Choose a reason for hiding this comment

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

:D

Now you're feeling that "unconfigurable configuration" vibe.


# GET /schools/new
def new
@school = School.new
end

# GET /schools/1/edit
def edit; end
def edit
end

# POST /schools or /schools.json
def create
@school = School.new(school_params)

respond_to do |format|
if @school.save
format.html { redirect_to school_url(@school), notice: 'School was successfully created.' }
format.html { redirect_to school_url(@school), notice: "School was successfully created." }
format.json { render :show, status: :created, location: @school }
else
format.html { render :new, status: :unprocessable_entity }
Expand All @@ -37,7 +39,7 @@ def create
def update
respond_to do |format|
if @school.update(school_params)
format.html { redirect_to school_url(@school), notice: 'School was successfully updated.' }
format.html { redirect_to school_url(@school), notice: "School was successfully updated." }
format.json { render :show, status: :ok, location: @school }
else
format.html { render :edit, status: :unprocessable_entity }
Expand All @@ -51,7 +53,7 @@ def destroy
@school.destroy!

respond_to do |format|
format.html { redirect_to schools_url, notice: 'School was successfully destroyed.' }
format.html { redirect_to schools_url, notice: "School was successfully destroyed." }
format.json { head :no_content }
end
end
Expand Down
3 changes: 2 additions & 1 deletion app/controllers/students_controller.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
class StudentsController < ApplicationController
before_action :set_student, only: %i[show]

def show; end
def show
end

private

Expand Down
2 changes: 1 addition & 1 deletion app/dashboards/classroom_dashboard.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class ClassroomDashboard < Administrate::BaseDashboard
users: Field::HasMany,
year: Field::BelongsTo,
created_at: Field::DateTime,
updated_at: Field::DateTime,
updated_at: Field::DateTime
}.freeze

# COLLECTION_ATTRIBUTES
Expand Down
2 changes: 1 addition & 1 deletion app/dashboards/portfolio_dashboard.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class PortfolioDashboard < Administrate::BaseDashboard
transactions: Field::String.with_options(searchable: false),
user: Field::BelongsTo,
created_at: Field::DateTime,
updated_at: Field::DateTime,
updated_at: Field::DateTime
}.freeze

# COLLECTION_ATTRIBUTES
Expand Down
2 changes: 1 addition & 1 deletion app/dashboards/portfolio_stock_dashboard.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class PortfolioStockDashboard < Administrate::BaseDashboard
shares: Field::Number.with_options(decimals: 2),
stock: Field::BelongsTo,
created_at: Field::DateTime,
updated_at: Field::DateTime,
updated_at: Field::DateTime
}.freeze

# COLLECTION_ATTRIBUTES
Expand Down
2 changes: 1 addition & 1 deletion app/dashboards/portfolio_transaction_dashboard.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
require 'administrate/base_dashboard'
require "administrate/base_dashboard"

class PortfolioTransactionDashboard < Administrate::BaseDashboard
# ATTRIBUTE_TYPES
Expand Down
2 changes: 1 addition & 1 deletion app/dashboards/school_dashboard.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class SchoolDashboard < Administrate::BaseDashboard
school_years: Field::HasMany,
years: Field::HasMany,
created_at: Field::DateTime,
updated_at: Field::DateTime,
updated_at: Field::DateTime
}.freeze

# COLLECTION_ATTRIBUTES
Expand Down
2 changes: 1 addition & 1 deletion app/dashboards/school_year_dashboard.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class SchoolYearDashboard < Administrate::BaseDashboard
school: Field::BelongsTo,
year: Field::BelongsTo,
created_at: Field::DateTime,
updated_at: Field::DateTime,
updated_at: Field::DateTime
}.freeze

# COLLECTION_ATTRIBUTES
Expand Down
2 changes: 1 addition & 1 deletion app/dashboards/stock_dashboard.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class StockDashboard < Administrate::BaseDashboard
stock_exchange: Field::String,
ticker: Field::String,
created_at: Field::DateTime,
updated_at: Field::DateTime,
updated_at: Field::DateTime
}.freeze

# COLLECTION_ATTRIBUTES
Expand Down
2 changes: 1 addition & 1 deletion app/dashboards/student_dashboard.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class StudentDashboard < Administrate::BaseDashboard
portfolio: Field::HasOne,
username: Field::String,
created_at: Field::DateTime,
updated_at: Field::DateTime,
updated_at: Field::DateTime
}.freeze

# COLLECTION_ATTRIBUTES
Expand Down
2 changes: 1 addition & 1 deletion app/dashboards/teacher_dashboard.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class TeacherDashboard < Administrate::BaseDashboard
portfolio: Field::HasOne,
username: Field::String,
created_at: Field::DateTime,
updated_at: Field::DateTime,
updated_at: Field::DateTime
}.freeze

# COLLECTION_ATTRIBUTES
Expand Down
2 changes: 1 addition & 1 deletion app/dashboards/user_dashboard.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class UserDashboard < Administrate::BaseDashboard
portfolio: Field::HasOne,
username: Field::String,
created_at: Field::DateTime,
updated_at: Field::DateTime,
updated_at: Field::DateTime
}.freeze

# COLLECTION_ATTRIBUTES
Expand Down
2 changes: 1 addition & 1 deletion app/dashboards/year_dashboard.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class YearDashboard < Administrate::BaseDashboard
schools: Field::HasMany,
year: Field::Number,
created_at: Field::DateTime,
updated_at: Field::DateTime,
updated_at: Field::DateTime
}.freeze

# COLLECTION_ATTRIBUTES
Expand Down
16 changes: 8 additions & 8 deletions app/helpers/components/input_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ def render_input(name:, label: false, id: nil, type: :text, value: nil, **option
options[:class] = tw(options[:class])

options.reverse_merge!(
label: (options[:lable] || false),
required: (options[:required] || false),
disabled: (options[:disabled] || false),
readonly: (options[:readonly] || false),
placeholder: (options[:placeholder] || ""),
autocomplete: (options[:autocomplete] || ""),
autocapitalize: (options[:autocapitalize] || nil),
autocorrect: (options[:autocorrect] || nil)
label: options[:lable] || false,
required: options[:required] || false,
disabled: options[:disabled] || false,
readonly: options[:readonly] || false,
placeholder: options[:placeholder] || "",
autocomplete: options[:autocomplete] || "",
autocapitalize: options[:autocapitalize] || nil,
autocorrect: options[:autocorrect] || nil
)
render partial: "components/ui/input", locals: {
type:,
Expand Down
2 changes: 1 addition & 1 deletion app/models/classroom.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
class Classroom < ApplicationRecord
belongs_to :year
belongs_to :school
has_many :users
has_many :users, dependent: :nullify
Copy link
Collaborator

Choose a reason for hiding this comment

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

I thought through the alternative arguments to dependent: and I think this is the correct one

Copy link
Member Author

Choose a reason for hiding this comment

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

I also suspect we may end up with a join model here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

🤔 Yeah, I can see that

end
2 changes: 1 addition & 1 deletion app/models/order.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@ class Order < ApplicationRecord
belongs_to :user
belongs_to :stock

enum status: { pending: 0, completed: 1, canceled: 2 }
enum status: {pending: 0, completed: 1, canceled: 2}
end
12 changes: 6 additions & 6 deletions app/models/portfolio.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
class Portfolio < ApplicationRecord
belongs_to :user
has_many :portfolio_transactions
has_many :portfolio_stocks
has_many :portfolio_transactions, dependent: :restrict_with_error
has_many :portfolio_stocks, dependent: :restrict_with_error

def cash_balance
portfolio_transactions.sum(:amount)
Expand All @@ -22,14 +22,14 @@ def buy_stock(ticker, shares)
# )
# self.update(cash_balance: self.cash_balance - cost) if transaction.persisted?

portfolio_stocks.create(stock:, shares:)
portfolio_stocks.create!(stock:, shares:)
end

# a User (specifically student) can sell a certain amount of stock
def sell_stock(ticker:, shares:)
stock = Stock.find_by(ticker:)
portfolio_stock = portfolio_stocks.find_by(stock_id: stock.id)
return unless portfolio_stock.present? # the stock we're trying to sell doesn't exist, return
return if portfolio_stock.blank? # the stock we're trying to sell doesn't exist, return

# update cash balance with portfolio_transactions

Expand All @@ -38,8 +38,8 @@ def sell_stock(ticker:, shares:)

# error, can't sell more than you have

portfolio_stock.update(shares: portfolio_stock.shares - shares)
portfolio_stock.destroy if portfolio_stock.shares == 0
portfolio_stock.update!(shares: portfolio_stock.shares - shares)
portfolio_stock.destroy! if portfolio_stock.shares == 0
end

# a User (usually teacher) can deposit money
Expand Down
2 changes: 1 addition & 1 deletion app/models/portfolio_stock.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@ class PortfolioStock < ApplicationRecord
def calculate_earnings
# based on purchase price
end
end
end
2 changes: 1 addition & 1 deletion app/models/portfolio_transaction.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
class PortfolioTransaction < ApplicationRecord
enum transaction_type: { deposit: 0, withdrawal: 1, credit: 2, debit: 3 }
enum transaction_type: {deposit: 0, withdrawal: 1, credit: 2, debit: 3}
belongs_to :portfolio

# Retrieve transactions occuring in a certain date range
Expand Down
2 changes: 1 addition & 1 deletion app/models/school.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
class School < ApplicationRecord
has_many :school_years
has_many :school_years, dependent: :destroy
has_many :years, through: :school_years
end
4 changes: 2 additions & 2 deletions app/models/stock.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
class Stock < ApplicationRecord
has_many :portfolio_stocks
has_many :orders
has_many :portfolio_stocks, dependent: :restrict_with_error
has_many :orders, dependent: :restrict_with_error

# Retreive general Stock information
def stock_information
Expand Down
2 changes: 1 addition & 1 deletion app/models/teacher.rb
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
class Teacher < User
end
end
8 changes: 4 additions & 4 deletions app/models/user.rb
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
class User < ApplicationRecord
has_one :portfolio
has_many :orders
has_one :portfolio, dependent: :nullify
has_many :orders, dependent: :nullify
belongs_to :classroom

# Include default devise modules. Others available are:
# :confirmable, :lockable, :timeoutable, :trackable and :omniauthable
devise :database_authenticatable, :registerable,
:recoverable, :rememberable, :validatable
:recoverable, :rememberable, :validatable

validates :email, uniqueness: true, presence: false, allow_blank: true
validates :email, uniqueness: true, presence: false, allow_blank: true # standard:disable Rails/UniqueValidationWithoutIndex
validates :username, presence: true, uniqueness: true

def email_required?
Expand Down
2 changes: 1 addition & 1 deletion app/models/year.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
class Year < ApplicationRecord
has_many :school_years
has_many :school_years, dependent: :nullify
has_many :schools, through: :school_years

validates :year, presence: true, uniqueness: true
Expand Down
Loading