Ponytech / appstoreconnectapi

Python wrapper around Apple App Store Api
https://ponytech.net/projects/app-store-connect
MIT License
161 stars 74 forks source link

Bad formatting in api.download_finance_reports() calls #21

Closed GClunies closed 4 years ago

GClunies commented 4 years ago

When I run the following code

from appstoreconnect import Api 

key_id = 'foo_bar'
path_to_key_file = './foo_bar.p8'
issuer_id = 'foo_bar'

api = Api(
    key_id,
    path_to_key_filee,
    issuer_id,
    submit_stats=False)

res1 = api.download_finance_reports(
    filters={
        'vendorNumber': '123456789',
        'reportDate': '2020-07',
        'reportType': 'FINANCIAL'
    },
    save_to='test.csv'
)

I get a poorly formatted .csv file. It looks like there is some strange text wrapping going on in the response from api.download_finance_reports(). The response always "wraps" at the Total Rows line. Both the reposnse and .csv show the same problem.

Start Date  End Date    UPC ISRC/ISBN   Vendor Identifier   Quantity    Partner Share   Extended Partner Share  Partner Share Currency  Sales or Return Apple Identifier    Artist/Show/Developer/Author    Title   Label/Studio/Network/Developer/Publisher    Grid    Product Type Identifier ISAN/Other Identifier   Country Of Sale Pre-order Flag  Promo Code  Customer Price  Customer Currency
03/29/2020  05/02/2020          foobar.iosapp.1month.subscription   1   44.52   44.52   AED S   123456789       foobar Premium Monthly          IAY     AE          54.99   AED
03/29/2020  05/02/2020          foobar.iosapp.1month.subscription   2   44.52   89.04   AED S   123456789       foobar Premium Monthly          IAY     AE          54.99   AED
03/29/2020  05/02/2020          foobar.iosapp.1month.subscription   2   36.66   73.32   AED S   123456789       foobar Premium Monthly          IAY     AE          54.99   AED
...
...
data continues in this pattern
...
...
Total_Rows  262
Country Of Sale Partner Share Currency  Quantity    Extended Partner Share
AE  AED 7   490.44
AU  AUD 471 14647.84
BR  BRL 113 7988.34

I am trying to work on a fix for this in my use case, but figured I would make you aware how it affects this packages functionality as well.

GClunies commented 4 years ago

@ppawlak Did some more digging into this strangely formatted response. It actually looks like the response itself contains 2 separate "pieces" of data.

Where the 1st piece is the data below, which corresponds to the description under the heading Financial report fields in the App Store Connect docs. In my case this has 262 rows of data (which is included in the response):

Start Date  End Date    UPC ISRC/ISBN   Vendor Identifier   Quantity    Partner Share   Extended Partner Share  Partner Share Currency  Sales or Return Apple Identifier    Artist/Show/Developer/Author    Title   Label/Studio/Network/Developer/Publisher    Grid    Product Type Identifier ISAN/Other Identifier   Country Of Sale Pre-order Flag  Promo Code  Customer Price  Customer Currency
03/29/2020  05/02/2020          foobar.iosapp.1month.subscription   1   44.52   44.52   AED S   123456789       foobar Premium Monthly          IAY     AE          54.99   AED
03/29/2020  05/02/2020          foobar.iosapp.1month.subscription   2   44.52   89.04   AED S   123456789       foobar Premium Monthly          IAY     AE          54.99   AED
03/29/2020  05/02/2020          foobar.iosapp.1month.subscription   2   36.66   73.32   AED S   123456789       foobar Premium Monthly          IAY     AE          54.99   AED
...
...
...
Total_Rows  262

The 2nd part of the response (below 'Total Rows` line) looks to be a financial summary by country (in my case 67 rows of data):

Country Of Sale Partner Share Currency  Quantity    Extended Partner Share
AE  AED 7   490.44
AU  AUD 471 14647.84
BR  BRL 113 7988.34
...
...
...

I think if you drop everything in the "raw" response after and including the line with Total_Rows then the response returned by api.download_finance_reports() would be the data that the end user is expecting. I'm achieving this right now by:

from appstoreconnect import Api 

key_id = 'foo_bar'
path_to_key_file = './foo_bar.p8'
issuer_id = 'foo_bar'

api = Api(
    key_id,
    path_to_key_filee,
    issuer_id,
    submit_stats=False)

res1 = api.download_finance_reports(
    filters={
        'vendorNumber': '12345678',
        'reportDate': '2020-07',
        'reportType': 'FINANCIAL'
    },
    save_to='test.csv'
)

def clean_finance_report_response(response):
    clean_response = response.split('Total_Rows')[0]

    return clean_response

res1_clean = clean_finance_report_response(res1)

print(res1_clean)
ppawlak commented 4 years ago

Hey @GClunies, thank you for this very detailed report and sorry for taking so long to reply.

I had a look at those report types and from what I saw the report downloaded by the API with 'reportType': 'FINANCIAL' is the All Countries or Regions (Single File) described here and which you can manually download on App Store Connect.

I was thinking about your idea to split the response and to save 2 separate CSVs files automatially. But I am worried about Apple changing the file format without notice, which I think they did in March 2020 according to the link above ("This report is available starting with your March 2020 report"). Splitting the response on a line that may not be present in the future will break this library, It may be safer to let library users do it themselves like you did. What do you think?

GClunies commented 4 years ago

No problem @ppawlak. Here is what I think:

  1. I wouldn't worry about Apple changing the file format (or API) since this will almost certainly happen at some point in the future. No point in losing sleep over it, you will just have to deal with it when it happens. I've had similar occurrences with packages I develop/maintain. I would instead focus on providing the best end-user experience when using appstoreconnect, given the current API and file formats.

  2. You could modify the api.download_finance_reports() function so that it defaults to a single response, but gives the user the option to return 2 separate responses. It could look something like...

    
    >> res1, res2 = api.download_finance_reports(
    filters={
        'vendorNumber': '12345678',
        'reportDate': '2020-07',
        'reportType': 'FINANCIAL'
    },
    split_response=True,  # Default is False (single response). When True, function returns 2 responses.
    )

print(res1) Start Date End Date UPC ISRC/ISBN Vendor Identifier Quantity Partner Share Extended Partner Share Partner Share Currency Sales or Return Apple Identifier Artist/Show/Developer/Author Title Label/Studio/Network/Developer/Publisher Grid Product Type Identifier ISAN/Other Identifier Country Of Sale Pre-order Flag Promo Code Customer Price Customer Currency 03/29/2020 05/02/2020 foobar.iosapp.1month.subscription 1 44.52 44.52 AED S 123456789 foobar Premium Monthly IAY AE 54.99 AED 03/29/2020 05/02/2020 foobar.iosapp.1month.subscription 2 44.52 89.04 AED S 123456789 foobar Premium Monthly IAY AE 54.99 AED 03/29/2020 05/02/2020 foobar.iosapp.1month.subscription 2 36.66 73.32 AED S 123456789 foobar Premium Monthly IAY AE 54.99 AED ... ... ...

print(res2) Country Of Sale Partner Share Currency Quantity Extended Partner Share AE AED 7 490.44 AU AUD 471 14647.84 BR BRL 113 7988.34 ... ... ...

You could then add some logic for the scenario when `split_response=True` but they also want to save the responses as `.csv`. Function call might then look like...
```python
res1, res2 = api.download_finance_reports(
filters={
'vendorNumber': '12345678',
'reportDate': '2020-07',
'reportType': 'FINANCIAL'
},
split_response=True,  # Default is False (single response). When True, function returns 2 responses.
save_to=['res1.csv', 'res2.csv'],
)

This would also address your concern of splitting the response on a line that may not be present in the future will break this library since the default would be to NOT split the response.

GClunies commented 4 years ago

If you don't mind @ppawlak, I can submit a PR to fix this if I have some time to play around with it later this week.

ppawlak commented 4 years ago

If you don't mind @ppawlak, I can submit a PR to fix this if I have some time to play around with it later this week.

I like your idea and the proposed function parameters. Please do!

ppawlak commented 4 years ago

Thank you very much for reporting and fixing this issue @GClunies !

It is published on pypi : https://pypi.org/project/appstoreconnect/0.8.2/