Rewrite of the dashboard reconciliation

[kaguillera]

This is a rewrite of the reconciliation report in the dashboard directory (gratipay/finances#21) It still needs to be tested on a snapshot of actual data to verify it works properly in production. Any advice on how I can recreate a snapshot of the database to do proper testing

[chadwhitacre]

Rebased on master at c213b9a. Previous head was fdd18d6.

[chadwhitacre]

Nice. Is this a Jinja2 thing?

[chadwhitacre]

Yes.

[chadwhitacre]

I see balanced-cc in 2012-07 as the first month and network. We should have all exchanges on this report, regardless of whether they have a route or not. A subhead for "no route" would actually be helpful in working through #3912.

[chadwhitacre]

Maybe http://stackoverflow.com/questions/2054235/why-doesnt-postgres-group-by-null-select-counts?

[kaguillera]

So these are the steps I think I have to make to show the null values in the dashboard reconciliation page:

• Create and branch.sql in local /sql/ folder to alter exchanges and payment_net
  • Alter exchange_status and payment_net by adding ‘ unknown’ type to enum
  • Insert records for ‘unknown’ network in exchange_route table
  • Update exchanges records with exchange_route ids pointing to ‘unknown’ network records for that participants
  • Update all null exchange_status to ‘ unknown'
  • Alter exchanges table and set status to not null
  • Alter exchange_routes table and set route to not null
• run and test changes made locally
• commit changes once satisfied it is working properly

[kaguillera]

This last commit is the changes made to database in order to capture the NULL values for route in the exchanges table that are not show on the reconciliation page. I tested it here but need real data as before so please review @whit537 as before... scratch I still have some worked based on the failed travis. I will look at it over the weekend.

[chadwhitacre]

Redid 35fc223 as c370b0d to uncomment SQL per IRL convo w/ @kaguillera.

[ghost]

@whit537 : is the reconciliation page supposed to be public, since it's in the dashboard? Just to be sure.

[chadwhitacre]

Yeah, if I remember right, earlier in the PR we had two sections on the reconciliation dashboard, one aggregate and one more detailed. The detailed one we wanted to protect, but I'm pretty sure it's gone now. We're okay with the aggregate numbers being public.

[ghost]

Ok, thanks for the reply ;-)

[kaguillera]

Finally started back working on this and realize(Trinidad/UK spelling) the reason for the error on Travis.
In fake_data.py the fake_exchange function does not associate the exchange with a fake_exchange_route. Thus to solve it I have to make the fake_exchange_routes for the fake participants and then associate (foreign_key reference) them with the fake exchanges that are created for that participant. So here goes...

[kaguillera]

Now this is finally up for review since travis is passing. ping @whit537

[chadwhitacre]

@kaguillera Ready for a rebase post-#3596, ya?

[kaguillera]

Let me just verify this and get back to you...

[kaguillera]

Finally ready for review and merging.

[chadwhitacre]

Need another rebase. 🐑

[chadwhitacre]

Talking through git wonkiness here. Trying to understand where the infamous "@kaguillera has added 250+ commits" is coming from. Planning to pick up tomorrow ...

[kaguillera]

🎉 No more conflicts 🎉 ready for review again

[chadwhitacre]

What's up with this reconciliation_new.spt file? Is it supposed to replace reconciliation.spt?

[kaguillera]

reconciliation_new.spt is a scratch file that I need to (and going to now) remove. Originally it was going to be a separate page but decided against it I actually thought that I had removed it.

[chadwhitacre]

Weird. Green again! O.O

[kaguillera]

Rebased

[kaguillera]

Ready again...what is the verdict...can we merge?

[chadwhitacre]

Goofy to use the string "None" here. I think the empty string might be a better choice?

[chadwhitacre]

There's a uniqueness constraint on (participant, network, address), which is why non-unique (network, address) is fine.

gratipay-bak=# \d exchange_routes
                                Table "public.exchange_routes"
┌─────────────┬───────────────┬──────────────────────────────────────────────────────────────┐
│   Column    │     Type      │                          Modifiers                           │
├─────────────┼───────────────┼──────────────────────────────────────────────────────────────┤
│ id          │ integer       │ not null default nextval('exchange_routes_id_seq'::regclass) │
│ participant │ bigint        │ not null                                                     │
│ network     │ payment_net   │ not null                                                     │
│ address     │ text          │ not null                                                     │
│ error       │ text          │ not null                                                     │
│ fee_cap     │ numeric(35,2) │                                                              │
└─────────────┴───────────────┴──────────────────────────────────────────────────────────────┘
Indexes:
    "exchange_routes_pkey" PRIMARY KEY, btree (id)
    "exchange_routes_participant_network_address_key" UNIQUE CONSTRAINT, btree (participant, network, address)
Check constraints:
    "exchange_routes_address_check" CHECK (address <> ''::text)
Foreign-key constraints:
    "exchange_routes_participant_fkey" FOREIGN KEY (participant) REFERENCES participants(id)
Referenced by:
    TABLE "exchanges" CONSTRAINT "exchanges_route_fkey" FOREIGN KEY (route) REFERENCES exchange_routes(id)

[kaguillera]

Soooo...if we replace the None for address and error will that be sufficient or do you still have some concerns

[chadwhitacre]

Yeah, let's switch to n/a for address and the empty string for error.

[chadwhitacre]

Done in bac13de.

[chadwhitacre]

Alright, I'm good here. I'm satisfied that we don't have code inserting new NULL routes or statuses, and I'm seeing this do what's intended in terms of converting existing NULLs to unknown. Deploying! 👍