From 25da517ad2568802f9615e96cf49d36e9930ca28 Mon Sep 17 00:00:00 2001
From: Vladislav Sokov <62177364+VladislavSokov@users.noreply.github.com>
Date: Thu, 7 Sep 2023 17:47:34 +0300
Subject: [PATCH] fix(public account share): fix public account share buttons
 and labels (#77)

* fix(public account share): fix public account share buttons and labels

* fix(public account share): fixup! fix public account share buttons and labels

* fix(public account share): fixup! fixup! fix public account share buttons and labels

* fix(public account share): fixup! fixup! fixup! fix public account share buttons and labels

---------

Co-authored-by: VladislavSokov <vladilsav.sokov.92@gmail.com>
---
 app/controllers/accounts_controller.rb        | 22 +-----
 app/controllers/my_accounts_controller.rb     |  4 -
 .../public_account_shares_controller.rb       | 17 ++++-
 app/models/account.rb                         |  4 +-
 app/models/account_share.rb                   |  3 +-
 .../account_shares/_account_share.html.slim   | 12 ++-
 app/views/accounts/_objective.html.slim       |  5 +-
 app/views/accounts/_transaction.html.slim     |  5 +-
 app/views/accounts/show.html.slim             | 33 ++++----
 app/views/my_accounts/show.html.slim          | 11 ---
 .../_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 +++-
 16 files changed, 155 insertions(+), 92 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 ef1aeaba..c01d6f9e 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
 
@@ -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
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/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.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 a8acb3bc..32c6f758 100644
--- a/app/models/account_share.rb
+++ b/app/models/account_share.rb
@@ -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
diff --git a/app/views/account_shares/_account_share.html.slim b/app/views/account_shares/_account_share.html.slim
index b5858fa6..0973a6fc 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', 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'
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'
-                    | &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'
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
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) }