feat(archive account): add account archiving feature #124
Conversation
| unless accessed? | ||
| flash.now.notice = 'Access denied!.' | ||
| return render :notification | ||
| end |
There was a problem hiding this comment.
it should just raise active record not found, i.e. fix the code so that it finds with "!" on a specific scope.
|
|
||
| account.update!(archived_at: Time.current) | ||
| AccountShare.where(account_id: account).delete_all | ||
| AccountAutomaticTopupConfig.where(to_account: account).delete_all |
There was a problem hiding this comment.
let's also archive AccountAutomaticTopupConfig, no need to delete data from DB
There was a problem hiding this comment.
@VladislavSokov I have a better idea. Let's not change anything except the account. However, the place that selects the top-up configurations should be changed to filter those related to deleted accounts.
app/models/account.rb
Outdated
| @@ -3,7 +3,7 @@ | |||
| class Account < ApplicationRecord | |||
| has_many :income_transactions, class_name: 'Transaction', foreign_key: :to_account_id | |||
| has_many :outcome_transactions, class_name: 'Transaction', foreign_key: :from_account_id | |||
| has_many :children, class_name: 'Account', foreign_key: :parent_id | |||
| has_many :children, -> { where(archived_at: nil) }, class_name: 'Account', foreign_key: :parent_id | |||
There was a problem hiding this comment.
create a scope unarchived and use it here and below.
|
|
||
| def accessed? | ||
| current_user.id == (account.user ? account.user.id : account.parent.user.id) | ||
| rescue ActiveRecord::RecordNotFound |
| return redirect_to account_path unless accessed? | ||
|
|
||
| account.update!(archived_at: Time.current) | ||
| AccountShare.where(account_id: account).delete_all |
There was a problem hiding this comment.
Why this is needed? We should change only the account here. All other related data, if it affects the system, should definitely be disabled. But it should be disabled only by filtering, e.g. checking if the related account is not archived.
There was a problem hiding this comment.
@VladislavSokov archived accounts should be excluded here too - https://github.com/widefix/pocketmoney/blob/9eab14933fbdca1820b99bd9cfc366d59035f700/lib/tasks/scheduler.rake#L4C5-L4C32
… archiving feature
…account archiving feature
| return redirect_to account_path unless accessed? | ||
|
|
||
| account.update!(archived_at: Time.current) |
There was a problem hiding this comment.
WDYT if we follow the same pattern with 404 here as well?
In this case, it's ok to not use the account helper method below but create a variable here:
account = Account.<scope>.find(ps.fetch(:id))
account.update!(archived_at: Time.current)
redirect_to my_account_path| @@ -1,4 +1,6 @@ | |||
| class AccountAutomaticTopupConfig < ApplicationRecord | |||
| belongs_to :from_account, class_name: 'Account', foreign_key: :from_account_id | |||
| belongs_to :to_account, class_name: 'Account', foreign_key: :to_account_id | |||
|
|
|||
| scope :active, -> { where(to_account: { archived_at: nil }) } | |||
There was a problem hiding this comment.
| scope :active, -> { where(to_account: { archived_at: nil }) } | |
| scope :visible, -> { where(to_account: { archived_at: nil }) } |
app/models/account_share.rb
Outdated
| @@ -7,6 +7,7 @@ class AccountShare < ApplicationRecord | |||
| scope :for, ->(user) { where(email: user.email) } | |||
| scope :accepted, -> { where.not(accepted_at: nil) } | |||
| scope :unaccepted, -> { where(accepted_at: nil) } | |||
| scope :active, -> { includes(:account).where(accounts: { archived_at: nil }) } | |||
There was a problem hiding this comment.
| scope :active, -> { includes(:account).where(accounts: { archived_at: nil }) } | |
| scope :visible, -> { includes(:account).where(accounts: { archived_at: nil }) } |
…p! add account archiving feature
No description provided.