From a6265d9145ab0960416b9887db69627013e90641 Mon Sep 17 00:00:00 2001 From: VladislavSokov Date: Thu, 7 Sep 2023 11:37:19 +0300 Subject: [PATCH 1/4] fix(public account share): fix public account share buttons and labels --- app/controllers/accounts_controller.rb | 4 ++++ app/controllers/my_accounts_controller.rb | 4 ---- app/views/account_shares/_account_share.html.slim | 12 ++++++++---- app/views/my_accounts/show.html.slim | 11 ----------- 4 files changed, 12 insertions(+), 19 deletions(-) diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index ef1aeaba..1bc325c8 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -1,4 +1,8 @@ +# frozen_string_literal: true + class AccountsController < ApplicationController + before_action :authenticate_user! + def show end diff --git a/app/controllers/my_accounts_controller.rb b/app/controllers/my_accounts_controller.rb index faa16367..f88ce3a2 100644 --- a/app/controllers/my_accounts_controller.rb +++ b/app/controllers/my_accounts_controller.rb @@ -15,10 +15,6 @@ def show; end Account.shared_for(current_user) end - helper_method memoize def public_shared_accounts - Account.shared_for_public - end - helper_method memoize def unaccepted_shares AccountShare.unaccepted.for(current_user) end diff --git a/app/views/account_shares/_account_share.html.slim b/app/views/account_shares/_account_share.html.slim index b5858fa6..4501338a 100644 --- a/app/views/account_shares/_account_share.html.slim +++ b/app/views/account_shares/_account_share.html.slim @@ -1,10 +1,14 @@ tr - td= account_share.name - td= account_share.email + - if account_share.public? + td= 'public' + td= 'public' + - else + td= account_share.name + td= account_share.email td= account_share.created_at.to_formatted_s(:short) td= account_share.accepted_at&.to_formatted_s(:short) - if account_share.public? - td= '' + td= link_to 'Link', account_path(account) - else td= link_to 'link', accept_account_share_url(token: account_share.token) - td= link_to 'Cancel', account_share_path(account, account_share), method: :delete, data: {confirm: 'Cancel this share?'}, class: 'button is-small is-light' + td= link_to 'Cancel', account_share_path(account, account_share), method: :delete, data: {confirm: 'Cancel this share?'}, class: 'button is-small is-light' diff --git a/app/views/my_accounts/show.html.slim b/app/views/my_accounts/show.html.slim index b0c9aa50..0959a40c 100644 --- a/app/views/my_accounts/show.html.slim +++ b/app/views/my_accounts/show.html.slim @@ -29,17 +29,6 @@ = link_to '+ Add', new_account_topup_path(shared_account), class: 'card-footer-item' = link_to '- Spend', new_account_spend_path(shared_account), class: 'card-footer-item' br - - if public_shared_accounts.present? - | Public shared accounts - - public_shared_accounts.each do |shared_account| - .card.box - .card-content - .media - .media-content - p.title.is-6 Owner: #{shared_account.parent.email} - p.title.is-4= link_to shared_account.name, account_path(shared_account) - p.subtitle.is-6= shared_account.balance - - if unaccepted_shares.present? h2.title | Unaccepted shares From 005e3dc35b110e8613f2b6c45175c8373cf599e4 Mon Sep 17 00:00:00 2001 From: VladislavSokov Date: Thu, 7 Sep 2023 12:07:12 +0300 Subject: [PATCH 2/4] fix(public account share): fixup! fix public account share buttons and labels --- app/controllers/accounts_controller.rb | 2 -- app/views/account_shares/_account_share.html.slim | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index 1bc325c8..dd04024c 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true class AccountsController < ApplicationController - before_action :authenticate_user! - def show end diff --git a/app/views/account_shares/_account_share.html.slim b/app/views/account_shares/_account_share.html.slim index 4501338a..1cb9e0eb 100644 --- a/app/views/account_shares/_account_share.html.slim +++ b/app/views/account_shares/_account_share.html.slim @@ -8,7 +8,7 @@ tr td= account_share.created_at.to_formatted_s(:short) td= account_share.accepted_at&.to_formatted_s(:short) - if account_share.public? - td= link_to 'Link', account_path(account) + td= link_to 'link', account_path(account) - else td= link_to 'link', accept_account_share_url(token: account_share.token) td= link_to 'Cancel', account_share_path(account, account_share), method: :delete, data: {confirm: 'Cancel this share?'}, class: 'button is-small is-light' From 7c14b37cda2cd454208d8b0369c262e08652fe25 Mon Sep 17 00:00:00 2001 From: VladislavSokov Date: Thu, 7 Sep 2023 17:26:33 +0300 Subject: [PATCH 3/4] fix(public account share): fixup! fixup! fix public account share buttons and labels --- app/controllers/accounts_controller.rb | 2 + .../public_account_shares_controller.rb | 17 ++++- app/models/account_share.rb | 2 +- .../account_shares/_account_share.html.slim | 2 +- .../_objective.html.slim | 7 ++ .../_transaction.html.slim | 7 ++ .../public_account_shares/show.html.slim | 75 +++++++++++++++++++ config/routes.rb | 1 + spec/controllers/accounts_spec.rb | 27 +++---- .../controllers/public_account_shares_spec.rb | 14 +++- 10 files changed, 130 insertions(+), 24 deletions(-) create mode 100644 app/views/public_account_shares/_objective.html.slim create mode 100644 app/views/public_account_shares/_transaction.html.slim create mode 100644 app/views/public_account_shares/show.html.slim diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index dd04024c..1bc325c8 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class AccountsController < ApplicationController + before_action :authenticate_user! + def show end diff --git a/app/controllers/public_account_shares_controller.rb b/app/controllers/public_account_shares_controller.rb index 2c0a1903..71b283c7 100644 --- a/app/controllers/public_account_shares_controller.rb +++ b/app/controllers/public_account_shares_controller.rb @@ -1,12 +1,23 @@ # frozen_string_literal: true class PublicAccountSharesController < ApplicationController - before_action :authenticate_user! + before_action :authenticate_user!, only: [:create, :new] + + def show; end def new; end def create - AccountShare.create!(user_id: current_user.id, account_id: account.id, accepted_at: Time.current) - redirect_to account_shares_path account + AccountShare.create!(user_id: current_user.id, + account_id: account.id, + token: SecureRandom.urlsafe_base64(32), + accepted_at: Time.current) + redirect_to account_shares_path(account) + end + + private + + helper_method memoize def public_account + AccountShare.find_by(token: params[:token]).account end end diff --git a/app/models/account_share.rb b/app/models/account_share.rb index a8acb3bc..6274993a 100644 --- a/app/models/account_share.rb +++ b/app/models/account_share.rb @@ -10,6 +10,6 @@ class AccountShare < ApplicationRecord scope :unaccepted, -> { where(accepted_at: nil) } memoize def public? - email.nil? && name.nil? && token.nil? + email.nil? && name.nil? end end diff --git a/app/views/account_shares/_account_share.html.slim b/app/views/account_shares/_account_share.html.slim index 1cb9e0eb..0973a6fc 100644 --- a/app/views/account_shares/_account_share.html.slim +++ b/app/views/account_shares/_account_share.html.slim @@ -8,7 +8,7 @@ tr td= account_share.created_at.to_formatted_s(:short) td= account_share.accepted_at&.to_formatted_s(:short) - if account_share.public? - td= link_to 'link', account_path(account) + td= link_to 'link', public_account_path(token: account_share.token) - else td= link_to 'link', accept_account_share_url(token: account_share.token) td= link_to 'Cancel', account_share_path(account, account_share), method: :delete, data: {confirm: 'Cancel this share?'}, class: 'button is-small is-light' diff --git a/app/views/public_account_shares/_objective.html.slim b/app/views/public_account_shares/_objective.html.slim new file mode 100644 index 00000000..1eabc168 --- /dev/null +++ b/app/views/public_account_shares/_objective.html.slim @@ -0,0 +1,7 @@ +tr + td= objective.name + td= objective.amount + td= number_to_percentage(public_account.balance / objective.amount * 100, precision: 0) + td= ([objective.amount - public_account.balance, 0].max / public_account.automatic_topup_configs.sum(:amount).+(0.1)).round(2) + td= image_tag objective.image_url + td= '' diff --git a/app/views/public_account_shares/_transaction.html.slim b/app/views/public_account_shares/_transaction.html.slim new file mode 100644 index 00000000..ad969bd9 --- /dev/null +++ b/app/views/public_account_shares/_transaction.html.slim @@ -0,0 +1,7 @@ +tr + td= transaction.date + td= transaction.from_account.name + td= transaction.to_account.name + td= transaction.signed_amount(public_account) + td= transaction.description + td= '' diff --git a/app/views/public_account_shares/show.html.slim b/app/views/public_account_shares/show.html.slim new file mode 100644 index 00000000..769351aa --- /dev/null +++ b/app/views/public_account_shares/show.html.slim @@ -0,0 +1,75 @@ +h2.title.is-4 + | Account details +.buttons + = link_to 'Back', my_account_path, class: 'button is-light' +.columns + .column + .card + .card-header + .card-header-title.is-centered + h3.title.is-4= public_account.name + .card-content + nav.level + div.level-item.has-text-centered + div + p.heading Balance + p.title #{public_account.balance} +.columns + .column + .card + .card-header + .card-header-title.is-centered + h3.title.is-4 History + .card-content.px-1 + .table-container + table.table.is-fullwidth.is-striped + thead + tr + th Date + th From + th To + th Amount + th Description + th Actions + tbody + = render partial: 'transaction', collection: public_account.transactions, public_account: public_account +.columns + .column + .card + .card-header + .card-header-title.is-centered + h3.title.is-4 Objectives + .card-content.px-1 + .table-container + table.table.is-fullwidth.is-striped + thead + tr + th Name + th Amount + th Progress + th Weeks # to goal + th Image + th Actions + tbody + = render partial: 'objective', collection: public_account.objectives, public_account: public_account +.columns + .column + .card + .card-header + .card-header-title.is-centered + .title.is-4 Weekly Automatic Topups + .card-content.px-1 + .table-container + table.table.is-fullwidth.is-striped + thead + tr + th From + th Amount + th Actions + - public_account.automatic_topup_configs.each do |config| + tr + td= config.from_account.name + td= config.amount + td '' +- if public_account.accumulative_balance_data.present? + = line_chart public_account.accumulative_balance_data, xtitle: 'Date', ytitle: 'Accumulative Balance' diff --git a/config/routes.rb b/config/routes.rb index f4db8a01..77b5bd4e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -16,4 +16,5 @@ resources :accept_account_shares, param: :token, only: %i[show update] resources :transactions, only: [:destroy] resources :objectives, only: [:destroy] + resources :public_accounts, param: :token, only: :show, controller: 'public_account_shares' end diff --git a/spec/controllers/accounts_spec.rb b/spec/controllers/accounts_spec.rb index cb1e4bda..d2f1b305 100644 --- a/spec/controllers/accounts_spec.rb +++ b/spec/controllers/accounts_spec.rb @@ -1,9 +1,14 @@ +# frozen_string_literal: true + require 'rails_helper' RSpec.describe AccountsController, type: :controller do - describe '#show' do - let(:account) { create(:account, :parent) } + let(:account) { create(:account, :parent) } + let(:user) { create(:user, account: account) } + before { sign_in user } + + describe '#show' do subject(:show) { get :show, params: {id: account } } it { is_expected.to have_http_status(:success) } @@ -20,26 +25,16 @@ end describe '#create' do - let(:parent) { create(:account, :parent) } - let(:user) { create(:user, account: parent) } - - subject { post :create, params: { parent: parent, name: parent.name } } - - before { sign_in user } + subject { post :create, params: { parent: account, name: account.name } } it { is_expected.to redirect_to(my_account_path) } it { is_expected.to have_http_status(302) } - it { expect { subject }.to change { Account.where(name: parent.name).count }.by(1) } + it { expect { subject }.to change { Account.where(name: account.name).count }.by(1) } it { expect { subject }.to change { Account.count }.by(1) } end describe '#edit' do - let(:account) { create(:account, :parent) } - let(:user) { create(:user, account: account) } - subject(:edit) { get :edit, params: { id: account }} - - before { sign_in user } it { is_expected.to have_http_status(:success) } it { is_expected.to render_template(:edit) } @@ -47,15 +42,11 @@ end describe '#update' do - let(:account) { create(:account, :parent) } - let(:user) { create(:user, account: account) } let(:name) { FFaker::Name.first_name } let(:old_name) { old_name = account.name } subject(:bad_update) { patch :update, params: { id: user.account, account: { name: nil }} } subject(:update) { patch :update, params: { id: user.account, account: { name: name }} } - - before { sign_in user } it { is_expected.to have_http_status(:redirect) } it { expect(bad_update).not_to be_redirect } diff --git a/spec/controllers/public_account_shares_spec.rb b/spec/controllers/public_account_shares_spec.rb index ae7890c4..35c27d41 100644 --- a/spec/controllers/public_account_shares_spec.rb +++ b/spec/controllers/public_account_shares_spec.rb @@ -6,9 +6,19 @@ let(:account) { create(:account, :parent) } let(:user) { create(:user, account: account) } - before { sign_in user } + describe '#show' do + let!(:account_share) { create(:account_share, user: user, account: account) } + + subject(:show) { get :show, params: { token: account_share.token } } + + it { is_expected.to have_http_status(:success) } + it { is_expected.to render_template(:show) } + it { is_expected.to_not render_template('home/index') } + end describe '#new' do + before { sign_in user } + subject(:new) { get :new, params: { account_id: account.id } } it { is_expected.to have_http_status(:success) } @@ -16,6 +26,8 @@ end describe '#create' do + before { sign_in user } + subject { post :create, params: { account_id: account.id } } it { is_expected.to redirect_to(account_shares_path) } From 9e4deb92481b12d2556740ec878aa1e2cb691973 Mon Sep 17 00:00:00 2001 From: VladislavSokov Date: Thu, 7 Sep 2023 17:36:04 +0300 Subject: [PATCH 4/4] fix(public account share): fixup! fixup! fixup! fix public account share buttons and labels --- app/controllers/accounts_controller.rb | 18 ------------- app/models/account.rb | 4 +-- app/models/account_share.rb | 1 - app/views/accounts/_objective.html.slim | 5 +--- app/views/accounts/_transaction.html.slim | 5 +--- app/views/accounts/show.html.slim | 33 +++++++++-------------- 6 files changed, 16 insertions(+), 50 deletions(-) diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index 1bc325c8..c01d6f9e 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -27,25 +27,7 @@ def create private - memoize def public_shared_accounts - Set.new(AccountShare.accepted.for_public.pluck(:account_id)) - end - - memoize def private_shared_accounts - Set.new(AccountShare.accepted.for(current_user).pluck(:account_id)) - end - helper_method memoize def account Account.visible_for(current_user).find(ps.fetch(:id)) end - - # An account isn't considered publicly shared if it's privately shared with the user - # or it's the child account of the current user. - - helper_method def public_share?(account) - return false if current_user.account_id == account.parent_id || - private_shared_accounts.include?(account.id) - - public_shared_accounts.include?(account.id) - end end diff --git a/app/models/account.rb b/app/models/account.rb index 519060ef..868dc139 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -19,12 +19,10 @@ class Account < ApplicationRecord scope :visible_for, lambda { |current_user| where(id: [current_user.account_id] + current_user.account.child_ids + - shared_for(current_user).pluck(:id) + - shared_for_public.pluck(:id)) + shared_for(current_user).pluck(:id)) } scope :shared_for, ->(user) { where(id: AccountShare.accepted.for(user).pluck(:account_id)) } - scope :shared_for_public, -> { where(id: AccountShare.accepted.for_public.pluck(:account_id)) } memoize def balance income_transactions.sum(:amount) - outcome_transactions.sum(:amount) diff --git a/app/models/account_share.rb b/app/models/account_share.rb index 6274993a..32c6f758 100644 --- a/app/models/account_share.rb +++ b/app/models/account_share.rb @@ -5,7 +5,6 @@ class AccountShare < ApplicationRecord belongs_to :account scope :for, ->(user) { where(email: user.email) } - scope :for_public, -> { where(email: nil) } scope :accepted, -> { where.not(accepted_at: nil) } scope :unaccepted, -> { where(accepted_at: nil) } diff --git a/app/views/accounts/_objective.html.slim b/app/views/accounts/_objective.html.slim index ae21f526..5766d78e 100644 --- a/app/views/accounts/_objective.html.slim +++ b/app/views/accounts/_objective.html.slim @@ -4,7 +4,4 @@ tr td= number_to_percentage(account.balance / objective.amount * 100, precision: 0) td= ([objective.amount - account.balance, 0].max / account.automatic_topup_configs.sum(:amount).+(0.1)).round(2) td= image_tag objective.image_url - - if public_share?(account) - td= '' - - else - td= link_to 'Delete', objective_path(objective), method: :delete, data: {confirm: 'Delete this objective?'}, class: 'button is-small is-light' + td= link_to 'Delete', objective_path(objective), method: :delete, data: {confirm: 'Delete this objective?'}, class: 'button is-small is-light' diff --git a/app/views/accounts/_transaction.html.slim b/app/views/accounts/_transaction.html.slim index d2beda61..38a579a5 100644 --- a/app/views/accounts/_transaction.html.slim +++ b/app/views/accounts/_transaction.html.slim @@ -4,7 +4,4 @@ tr td= transaction.to_account.name td= transaction.signed_amount(account) td= transaction.description - - if public_share?(account) - td= '' - - else - td= link_to 'Cancel', transaction_path(transaction), method: :delete, data: {confirm: 'Cancel this transaction?'}, class: 'button is-small is-light' + td= link_to 'Cancel', transaction_path(transaction), method: :delete, data: {confirm: 'Cancel this transaction?'}, class: 'button is-small is-light' diff --git a/app/views/accounts/show.html.slim b/app/views/accounts/show.html.slim index d50e1726..59fdbce4 100644 --- a/app/views/accounts/show.html.slim +++ b/app/views/accounts/show.html.slim @@ -2,9 +2,8 @@ h2.title.is-4 | Account details .buttons = link_to 'Back', my_account_path, class: 'button is-light' - - unless public_share?(account) - = link_to 'Edit', edit_account_path(account), class: 'button is-light' - = link_to 'Account shares', account_shares_path(account), class: 'button is-light' + = link_to 'Edit', edit_account_path(account), class: 'button is-light' + = link_to 'Account shares', account_shares_path(account), class: 'button is-light' .columns .column .card @@ -17,10 +16,9 @@ h2.title.is-4 div p.heading Balance p.title #{account.balance} - - unless public_share?(account) - .card-footer - = link_to '+Add', new_account_topup_path(account), class: 'card-footer-item' - = link_to '-Spend', new_account_spend_path(account), class: 'card-footer-item' + .card-footer + = link_to '+Add', new_account_topup_path(account), class: 'card-footer-item' + = link_to '-Spend', new_account_spend_path(account), class: 'card-footer-item' .columns .column .card @@ -59,9 +57,8 @@ h2.title.is-4 th Actions tbody = render partial: 'objective', collection: account.objectives, account: account - - unless public_share?(account) - .card-footer - = link_to '+Add', new_account_objective_path(account), class: 'card-footer-item' + .card-footer + = link_to '+Add', new_account_objective_path(account), class: 'card-footer-item' .columns .column .card @@ -80,15 +77,11 @@ h2.title.is-4 tr td= config.from_account.name td= config.amount - - if public_share?(account) - td '' - - else - td - = link_to 'Edit', edit_account_account_automatic_topup_config_path(account, config), class: 'button is-small is-light' - |   - = link_to 'Cancel', account_account_automatic_topup_config_path(account, config), method: :delete, data: { confirm: 'Cancel this top up?' }, class: 'button is-small is-light' - - unless public_share?(account) - .card-footer - = link_to '+Add', new_account_account_automatic_topup_config_path(account), class: 'card-footer-item' + td + = link_to 'Edit', edit_account_account_automatic_topup_config_path(account, config), class: 'button is-small is-light' + |   + = link_to 'Cancel', account_account_automatic_topup_config_path(account, config), method: :delete, data: { confirm: 'Cancel this top up?' }, class: 'button is-small is-light' + .card-footer + = link_to '+Add', new_account_account_automatic_topup_config_path(account), class: 'card-footer-item' - if account.accumulative_balance_data.present? = line_chart account.accumulative_balance_data, xtitle: 'Date', ytitle: 'Accumulative Balance'