Skip to content

Rename/refactor in models for clarity #533

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.

Sign up for GitHub

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

Open
NoRePercussions wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Rename/refactor in models for clarity #533

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
eff2b85
Properly name class representing academic year
NoRePercussions Nov 16, 2023
31679eb
Add intended effect to validator
NoRePercussions Nov 17, 2023
373fdd0
Clear up code for events model
NoRePercussions Nov 17, 2023
ae5e9c3
Cleaning event roles model
NoRePercussions Nov 17, 2023
99e8f7a
Clean event roles application
NoRePercussions Nov 17, 2023
f1bc98a
Add commentary and minor edits to event date model
NoRePercussions Nov 17, 2023
5e37928
Rearrange sorting logic
NoRePercussions Nov 17, 2023
3244e71
Small name change to member
NoRePercussions Nov 17, 2023
0abc0ac
Organize out clever logic from timecard entries
NoRePercussions Nov 17, 2023
da8c3da
Fix sort for invalid objects
NoRePercussions Feb 13, 2024
77bb967
Clarify changes to equipment profile:
NoRePercussions Feb 13, 2024
9d10003
Fix typo
NoRePercussions Feb 13, 2024
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
6 changes: 3 additions & 3 deletions app/models/account.rb → app/models/current_academic_year.rb
View file
Open in desktop
Copy link
Copy Markdown
Member

@DaAwesomeP DaAwesomeP Jan 17, 2024

Choose a reason for hiding this comment

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

I know that we don't run the tests, but there are some files in test/ that also need a rename (for consistency and to prevent later confusion).

Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
class Account
class CurrentAcademicYear

def self.magic_date
def self.start_date
if Date.today.month < 7
(Date.today.year - 1).to_s + '-07-01'
else
Date.today.year.to_s + '-07-01'
end
end

def self.future_magic_date
def self.end_date
if Date.today.month < 7
Date.today.year.to_s + '-07-01'
else
Expand Down
6 changes: 3 additions & 3 deletions app/models/event.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class Event < ActiveRecord::Base
# validate :eventdate_valid?
validate :textable_social_valid?

scope :current_year, -> { where("representative_date >= ? or last_representative_date > ?", Account.magic_date, Account.magic_date) }
scope :current_year, -> { where("representative_date >= ? or last_representative_date > ?", CurrentAcademicYear.start_date, CurrentAcademicYear.start_date) }

ThinkingSphinx::Callbacks.append(self, :behaviours => [:sql, :deltas]) # associated via eventdate

Expand Down Expand Up @@ -158,7 +158,7 @@ def current_year?
# a part of both years (i.e. Precollege). Otherwise Tracker does not allow
# certain functions (like invoicing) for the now previous year's event. So,
# we considered an event by start and end.
(representative_date >= Account.magic_date) or (self.last_representative_date > Account.magic_date)
(representative_date >= CurrentAcademicYear.start_date) or (self.last_representative_date > CurrentAcademicYear.start_date)
end

private
Expand Down Expand Up @@ -220,7 +220,7 @@ def set_created_email
end

def eventdate_valid?
if representative_date < Account.magic_date
if representative_date < CurrentAcademicYear.start_date
errors.add(:representative_date, "Requested date out of range")
end
end
Expand Down
2 changes: 1 addition & 1 deletion app/models/timecard.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class Timecard < ApplicationRecord

def self.valid_eventdates
timecards = self.valid_timecards
return Eventdate.where(["startdate >= ? AND events.billable = ?", Account.magic_date, true]).includes(:event).references(:event).sort_by{|ed| ed.event.title} if timecards.size == 0
return Eventdate.where(["startdate >= ? AND events.billable = ?", CurrentAcademicYear.start_date, true]).includes(:event).references(:event).sort_by{|ed| ed.event.title} if timecards.size == 0
start_date, end_date = timecards.inject([nil,nil]) do |pair, timecard|
[
((pair[0].nil? or timecard.start_date < pair[0]) ? timecard.start_date : pair[0]),
Expand Down
4 changes: 2 additions & 2 deletions db/migrate/20140114010016_fix_timezones.rb
View file
Open in desktop
Copy link
Copy Markdown
Member

@DaAwesomeP DaAwesomeP Jan 17, 2024

Choose a reason for hiding this comment

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

Do not edit old migrations. You should create a new migration that renames the table/model. Performing this new rename migration before you commit will also update the schema.

Copy link
Copy Markdown
Contributor Author

@NoRePercussions NoRePercussions Feb 13, 2024

Choose a reason for hiding this comment

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

Migration changes will be reverted.

The accounts table itself never got deleted/had its entries removed, just no longer is used. If this information is used anymore, a rename in the database would be unclear, and removing columns would remove the data. Perhaps it would be better to leave it be or remove it, and add a new helper class which does the academic year?

Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
class FixTimezones < ActiveRecord::Migration
def up
Account.all.each do |account|
CurrentAcademicYear.all.each do |account|
account.update_column(:created_at, account.created_at - account.created_at.utc_offset) if account.created_at
account.update_column(:updated_at, account.updated_at - account.updated_at.utc_offset) if account.updated_at
end
Expand Down Expand Up @@ -121,7 +121,7 @@ def up
end

def down
Account.all.each do |account|
CurrentAcademicYear.all.each do |account|
account.update_column(:created_at, account.created_at + account.created_at.utc_offset) if account.created_at
account.update_column(:updated_at, account.updated_at + account.updated_at.utc_offset) if account.updated_at
end
Expand Down
4 changes: 2 additions & 2 deletions db/migrate/20150221221612_convert_tables_to_unicode.rb
View file
Open in desktop
Copy link
Copy Markdown
Member

@DaAwesomeP DaAwesomeP Jan 17, 2024

Choose a reason for hiding this comment

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

(see first migration file comment)

Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ class ConvertTablesToUnicode < ActiveRecord::Migration
def up
execute "ALTER DATABASE " + ActiveRecord::Base.connection.current_database + " CHARACTER SET utf8 COLLATE utf8_unicode_ci;"

models = [Account, Attachment, Blackout, Comment, EmailForm, Email, EquipmentCategory, EquipmentEventdate, Equipment, EventRole, Event, Eventdate, InvoiceItem, InvoiceLine, Invoice, Journal, Location, Member, Organization, TimecardEntry, Timecard]
models = [CurrentAcademicYear, Attachment, Blackout, Comment, EmailForm, Email, EquipmentCategory, EquipmentEventdate, Equipment, EventRole, Event, Eventdate, InvoiceItem, InvoiceLine, Invoice, Journal, Location, Member, Organization, TimecardEntry, Timecard]
models.each do |m|
tn = m.table_name
execute "ALTER TABLE " + tn + " CONVERT TO CHARACTER SET utf8 COLLATE utf8_unicode_ci;"
Expand All @@ -28,7 +28,7 @@ def up
def down
execute "ALTER DATABASE " + ActiveRecord::Base.connection.current_database + " CHARACTER SET latin1 COLLATE latin1_swedish_ci;"

models = [Account, Attachment, Blackout, Comment, EmailForm, Email, EquipmentCategory, EquipmentEventdate, Equipment, EventRole, Event, Eventdate, InvoiceItem, InvoiceLine, Invoice, Journal, Location, Member, Organization, TimecardEntry, Timecard]
models = [CurrentAcademicYear, Attachment, Blackout, Comment, EmailForm, Email, EquipmentCategory, EquipmentEventdate, Equipment, EventRole, Event, Eventdate, InvoiceItem, InvoiceLine, Invoice, Journal, Location, Member, Organization, TimecardEntry, Timecard]
models.each do |m|
tn = m.table_name
execute "ALTER TABLE " + tn + " CONVERT TO CHARACTER SET latin1 COLLATE latin1_swedish_ci;"
Expand Down
4 changes: 2 additions & 2 deletions db/migrate/20160815162731_convert_tables_to_real_unicode.rb
View file
Open in desktop
Copy link
Copy Markdown
Member

@DaAwesomeP DaAwesomeP Jan 17, 2024

Choose a reason for hiding this comment

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

(see first migration file comment)

Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ def up

execute "ALTER DATABASE " + ActiveRecord::Base.connection.current_database + " CHARACTER SET utf8mb4 COLLATE utf8mb4_bin;"

models = [Account, Attachment, Blackout, Comment, EmailForm, Email, Equipment, EventRoleApplication, EventRole, Event, Eventdate, InvoiceItem, InvoiceLine, Invoice, Journal, Location, Member, Organization, SuperTic, TimecardEntry, Timecard]
models = [CurrentAcademicYear, Attachment, Blackout, Comment, EmailForm, Email, Equipment, EventRoleApplication, EventRole, Event, Eventdate, InvoiceItem, InvoiceLine, Invoice, Journal, Location, Member, Organization, SuperTic, TimecardEntry, Timecard]
models.each do |m|
tn = m.table_name
execute "ALTER TABLE " + tn + " CONVERT TO CHARACTER SET utf8mb4 COLLATE utf8mb4_bin;"
Expand Down Expand Up @@ -49,7 +49,7 @@ def up
def down
execute "ALTER DATABASE " + ActiveRecord::Base.connection.current_database + " CHARACTER SET utf8 COLLATE utf8_unicode_ci;"

models = [Account, Attachment, Blackout, Comment, EmailForm, Email, Equipment, EventRoleApplication, EventRole, Event, Eventdate, InvoiceItem, InvoiceLine, Invoice, Journal, Location, Member, Organization, SuperTic, TimecardEntry, Timecard]
models = [CurrentAcademicYear, Attachment, Blackout, Comment, EmailForm, Email, Equipment, EventRoleApplication, EventRole, Event, Eventdate, InvoiceItem, InvoiceLine, Invoice, Journal, Location, Member, Organization, SuperTic, TimecardEntry, Timecard]
models.each do |m|
tn = m.table_name
execute "ALTER TABLE " + tn + " CONVERT TO CHARACTER SET utf8 COLLATE utf8_unicode_ci;"
Expand Down