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(public account share): fix public account share buttons and labels #77

Merged
merged 4 commits into from
Sep 7, 2023
Merged
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
22 changes: 4 additions & 18 deletions app/controllers/accounts_controller.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
# frozen_string_literal: true

class AccountsController < ApplicationController
before_action :authenticate_user!
Copy link
Member

Choose a reason for hiding this comment

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

The public shared account should be available for all people (including not logged in). That's why they are public.


def show
end

Expand All @@ -23,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
4 changes: 0 additions & 4 deletions app/controllers/my_accounts_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 14 additions & 3 deletions app/controllers/public_account_shares_controller.rb
Original file line number Diff line number Diff line change
@@ -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
4 changes: 1 addition & 3 deletions app/models/account.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 1 addition & 2 deletions app/models/account_share.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,10 @@ 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) }

memoize def public?
email.nil? && name.nil? && token.nil?
email.nil? && name.nil?
end
end
12 changes: 8 additions & 4 deletions app/views/account_shares/_account_share.html.slim
Original file line number Diff line number Diff line change
@@ -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', 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'
td= link_to 'Cancel', account_share_path(account, account_share), method: :delete, data: {confirm: 'Cancel this share?'}, class: 'button is-small is-light'
5 changes: 1 addition & 4 deletions app/views/accounts/_objective.html.slim
Original file line number Diff line number Diff line change
Expand Up @@ -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'
5 changes: 1 addition & 4 deletions app/views/accounts/_transaction.html.slim
Original file line number Diff line number Diff line change
Expand Up @@ -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'
33 changes: 13 additions & 20 deletions app/views/accounts/show.html.slim
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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'
| &nbsp;
= 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'
| &nbsp;
= 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'
11 changes: 0 additions & 11 deletions app/views/my_accounts/show.html.slim
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions app/views/public_account_shares/_objective.html.slim
Original file line number Diff line number Diff line change
@@ -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= ''
7 changes: 7 additions & 0 deletions app/views/public_account_shares/_transaction.html.slim
Original file line number Diff line number Diff line change
@@ -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= ''
75 changes: 75 additions & 0 deletions app/views/public_account_shares/show.html.slim
Original file line number Diff line number Diff line change
@@ -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'
1 change: 1 addition & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
27 changes: 9 additions & 18 deletions spec/controllers/accounts_spec.rb
Original file line number Diff line number Diff line change
@@ -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) }
Expand All @@ -20,42 +25,28 @@
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) }
it { is_expected.to_not render_template(:new) }
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 }
Expand Down
Loading