Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use original headers on csv downloads #4143

Merged
merged 12 commits into from
Apr 5, 2024
Merged

Use original headers on csv downloads #4143

merged 12 commits into from
Apr 5, 2024

Conversation

janette
Copy link
Member

@janette janette commented Mar 8, 2024

Replaces machine names with original CSV headers in CSV download.

QA Steps

  • On a demo site, try the following request:
POST https://csvdown.ddev.site/api/1/datastore/query/cedcd327-4e5d-43f9-8eb1-c11850fa7c55/0

{
    "format": "csv",
    "properties": [
        "roadway"    
    ],
    "limit": 10
}

Note that the column header is ROADWAY.

  • Now try with an expression:
POST https://csvdown.ddev.site/api/1/datastore/query/cedcd327-4e5d-43f9-8eb1-c11850fa7c55/0

{
    "format": "csv",
    "properties": [
        "roadway",
        {
            "expression": {
                "operator": "+",
                "operands": [
                    "objectid",
                    1
                ]
            },
            "alias": "objectid_plus_1"
        }
    ],
    "limit": 10
}

Note that ROADWAY is shown in caps, and alias is preserved.

If you want to try a join, try this but you will need to find the correct distribution IDs:

POST https://csvdown.ddev.site/api/1/datastore/query

{
    "format": "csv",
    "resources": [
        {
            "id": "5733123b-6b70-5ff2-8876-c4e3a7c58398",
            "alias": "t"
        },
        {
            "id": "a180fe3c-e88d-55d9-841f-912ab0c4362d",
            "alias": "s"
        }
    ],
    "joins": [
        {
            "resource": "s",
            "condition": {
                "resource": "t",
                "property": "record_number",
                "value": {
                    "resource": "s",
                    "property": "record_number"
                }
            }
        }
    ],
    "properties": [
        "roadway",
        {
            "resource": "s",
            "property": "city"
        },
        {
            "resource": "s",
            "property": "violent_crimes"
        } 
    ],
    "limit": 10
}

Known issues

  • If schema property in request is set to false, the machine names will be shown. This is arguably expected behavior? The "schema" property in the request will be ignored if the format is "csv", and forced to true behind the scenes.
  • In the case of a join, the properties from the second resource will have their machine names. Decided this was out of scope for this PR and will fix in the future.
  • If "rowIds" is set to TRUE, adding the record_number column, we will still return machine names for all other columns as well.
  • In general a lot of the CSV header logic relies on information pulled from the query or results objects which seems less than ideal.

@dafeder dafeder marked this pull request as ready for review April 4, 2024 18:52
@dafeder dafeder requested a review from paul-m April 4, 2024 20:02
Copy link
Contributor

@paul-m paul-m left a comment

Choose a reason for hiding this comment

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

Looks like everything works... I exported and it used the CSV column names rather than the table column names. Just a few nitpicky things here about method names.

modules/datastore/src/Controller/QueryController.php Outdated Show resolved Hide resolved
modules/datastore/src/Service/Query.php Outdated Show resolved Hide resolved
@paul-m paul-m merged commit cd39b31 into 2.x Apr 5, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants