Closed renatomassaro closed 6 years ago
Ebert has finished reviewing this Pull Request and has found:
You can see more details about this review at https://ebertapp.io/github/HackerExperience/Helix/pulls/386.
Reviewed 55 of 55 files at r1. Review status: all files reviewed at latest revision, 12 unresolved discussions.
lib/network/henforcer/bounce.ex, line 113 at r1 (raw file):
end # REVIEW: Why the empty map?
Review
lib/software/action/virus.ex, line 34 at r1 (raw file):
""" def collect(file, payment_info) do virus = VirusQuery.fetch(file.file_id)
Add this into the with
lib/software/internal/virus.ex, line 134 at r1 (raw file):
result = force_activate_virus(virus, storage_id) # Review: really? because comment says only `:nothing`
Review
lib/software/model/virus.ex, line 104 at r1 (raw file):
end def calculate_earnings(
doc + spec
lib/software/model/virus.ex, line 107 at r1 (raw file):
_file = %File{}, _virus = %Virus{is_active?: true}, _saved_earnings) do # Obviously TODO
Add issue about global Balance module and reference it
lib/software/process/virus/collect.ex, line 125 at r1 (raw file):
end target_bank_account(_, _, _, %{virus: %{software_type: :virus_miner}}) do
doc
lib/software/websocket/requests/virus/collect.ex, line 20 at r1 (raw file):
def check_params(request, socket) do check_account_info =
doc
lib/software/websocket/requests/virus/collect.ex, line 42 at r1 (raw file):
true <- valid_bank_info?(atm_id, account_number), # Viruses must not be an empty list
Use backquotes
lib/software/websocket/requests/virus/collect.ex, line 138 at r1 (raw file):
render_empty() defp valid_bank_info?(nil, nil),
spec
lib/software/websocket/requests/virus/collect.ex, line 145 at r1 (raw file):
do: false defp valid_payment_info?({nil, nil}, nil),
spec
test/support/software/setup/virus.ex, line 33 at r1 (raw file):
end virus =
doc the extra conditional
test/universe/bank/henforcer/bank_test.exs, line 25 at r1 (raw file):
test "rejects when account does not exist" do
extra line
Comments from Reviewable
Reviewed 10 of 10 files at r2. Review status: all files reviewed at latest revision, all discussions resolved.
Comments from Reviewable
when will we next hear about an update for he2?
This PR is meant to allow players to collect money off of active viruses and have the earnings transferred to their bank account / bitcoin wallet.
collect/2
on VirusActionVirusHenforcer
Incidental
src_atm_id
,src_acc_number
,tgt_atm_id
andtgt_acc_number
were added to the Process structBankHenforcer
andowns_bank_account?
check onEntityHenforcer
BankAccountUpdatedEvent
(part of #361)BankAction.direct_deposit/2
order
macro onHELL.Ecto.Macros
This change is