Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

Fix bug in MassPay vis-a-vis refs #4537

Merged
merged 3 commits into from
Jul 6, 2017
Merged

Fix bug in MassPay vis-a-vis refs #4537

merged 3 commits into from
Jul 6, 2017

Conversation

chadwhitacre
Copy link
Contributor

@chadwhitacre chadwhitacre commented Jul 6, 2017

@chadwhitacre
Copy link
Contributor Author

Denied payments have an empty transaction id. Evident in the log and appearing under inspection.

diff --git a/bin/masspay.py b/bin/masspay.py
index 99d7f8d..01715cd 100755
--- a/bin/masspay.py
+++ b/bin/masspay.py
@@ -179,8 +179,12 @@ def load_statuses_and_refs():
         if line.startswith('Transaction ID,Recipient'):
             break
     for rec in csv.reader(fp):
-        statuses[rec[1]] = _status_map[rec[5]]
-        refs[rec[1]] = rec[0]
+        ref = rec[0]
+        email = rec[1]
+        status = rec[5]
+        statuses[email] = _status_map[status]
+        refs[email] = ref
+    import pdb; pdb.set_trace()
     return statuses, refs

@chadwhitacre
Copy link
Contributor Author

The MassPay itself has a unique transaction ID. We don't have access to that?

screen shot 2017-07-06 at 7 58 49 am

@chadwhitacre
Copy link
Contributor Author

No, doesn't appear in the log.

We could manually add before posting masspay back, but we currently require a unique ref (if not NULL).

@chadwhitacre
Copy link
Contributor Author

We could munge it to be unique. 1234567898765-a or whatever.

@chadwhitacre
Copy link
Contributor Author

Ready for review @rohitpaulk @clone1018.

@chadwhitacre
Copy link
Contributor Author

This requires the payday runner to paste in the unique transaction id for the MassPay. I'll update the docs ...

bin/masspay.py Outdated
for i, rec in enumerate(csv.reader(fp)):
ref = rec[0]
if not ref:
ref = '{}-{:>04}'.format(masspay_id, i)
Copy link
Contributor

Choose a reason for hiding this comment

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

Worth adding a comment that says why we're doing this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in dda94ad.

@chadwhitacre
Copy link
Contributor Author

chadwhitacre commented Jul 6, 2017

Doc update in gratipay/inside.gratipay.com#1113.

@chadwhitacre
Copy link
Contributor Author

This requires the payday runner to paste in the unique transaction id for the MassPay.

Actually, this appears to not be true.

gratipay::DATABASE=> \d exchanges
                                     Table "public.exchanges"
┌─────────────┬──────────────────────────┬────────────────────────────────────────────────────────┐
│   Column    │           Type           │                       Modifiers                        │
├─────────────┼──────────────────────────┼────────────────────────────────────────────────────────┤
│ id          │ integer                  │ not null default nextval('exchanges_id_seq'::regclass) │
│ timestamp   │ timestamp with time zone │ not null default now()                                 │
│ amount      │ numeric(35,2)            │ not null                                               │
│ fee         │ numeric(35,2)            │ not null                                               │
│ participant │ text                     │ not null                                               │
│ recorder    │ text                     │                                                        │
│ note        │ text                     │                                                        │
│ status      │ exchange_status          │ not null                                               │
│ route       │ bigint                   │ not null                                               │
│ ref         │ text                     │                                                        │
└─────────────┴──────────────────────────┴────────────────────────────────────────────────────────┘
Indexes:
    "exchanges_pkey" PRIMARY KEY, btree (id)
Foreign-key constraints:
    "exchanges_participant_fkey" FOREIGN KEY (participant) REFERENCES participants(username) ON UPDATE CASCADE ON DELETE RESTRICT
    "exchanges_recorder_fkey" FOREIGN KEY (recorder) REFERENCES participants(username) ON UPDATE CASCADE ON DELETE RESTRICT
    "exchanges_route_fkey" FOREIGN KEY (route) REFERENCES exchange_routes(id)

@chadwhitacre
Copy link
Contributor Author

chadwhitacre commented Jul 6, 2017

I guess it's because it wants to be unique per network, and that's in a different table (exchange_routes)?

@chadwhitacre
Copy link
Contributor Author

Gonna run this per @rohitpaulk in slack:

Looks good to me, aside from adding a small comment

Will leave the merge though ...

@kaguillera
Copy link
Contributor

And it [ref] is needed because want to have a track it. This is part of backfill

@chadwhitacre
Copy link
Contributor Author

Yep. Backfill being #4442.

@rohitpaulk rohitpaulk merged commit 231af79 into master Jul 6, 2017
@chadwhitacre chadwhitacre deleted the fix-masspay-refs branch July 6, 2017 16:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants