changes to support non-primary key in Piccolo Admin

[sinisaos]

Related to Supporting foreign keys which reference non-primary keys #144

[codecov-commenter]

Codecov Report

Merging #134 (2cd11bb) into master (92ea0ac) will increase coverage by 0.04%. The diff coverage is 100.00%.

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##           master     #134      +/-   ##
==========================================
+ Coverage   92.55%   92.59%   +0.04%     
==========================================
  Files          33       33              
  Lines        2027     2039      +12     
==========================================
+ Hits         1876     1888      +12     
  Misses        151      151              

Impacted Files	Coverage Δ
piccolo_api/crud/endpoints.py	94.89% <100.00%> (+0.12%)	:arrow_up:

[dantownsend]

@sinisaos I see where you're going with this. I'll have another look tomorrow at the UI side.

[sinisaos]

@dantownsend Basically we have a working list of rows with only primary keys, only with non-primary keys, or a mixture of both. The problem is the filter search in sidebar and referencing tables and how to combine id and readable based on target_colum or primary key. I hope you got some ideas. This is tricky with a lot of edge cases.

[sinisaos]

@dantownsend After a lot of experimentation I managed to solve the filter search problem to some extent. My approach is to always use a primary key even if table has a non-primary key FK relation to table. Everything works if it only has a non-primary key or a combination of primary key and non-primary key. Also tests for that need to be written. The only situation when it doesn't work well is when we have more than one non-primary key per table

class Review(Table):
    reviewer = ForeignKey(Director, target_column=Director.name)
    ...
    movie = ForeignKey(Movie, target_column=Movie.name)

because the loop in _apply_filters returns only the last result.

For referencing table I took same approach and we need to make this changes in Piccolo Admin

// changes in ReferencingTables.vue
<script lang="ts">
import Vue from "vue"
import { Location } from "vue-router"
import { TableReferencesAPIResponse, TableReference } from "../interfaces"

export default Vue.extend({
    props: ["tableName", "rowID"],
    data() {
        return {
            references: [] as TableReference[],
            showReferencing: false
        }
    },
    computed: {
        selectedRow() {
            return this.$store.state.selectedRow
        },
        schema() {
            return this.$store.state.schema
        }
    },
    methods: {
        async fetchTableReferences() {
            const response = await this.$store.dispatch(
                "fetchTableReferences",
                this.tableName
            )

            this.references = (
                response.data as TableReferencesAPIResponse
            ).references
        },
        clickedReference(reference: TableReference) {
            let columnName = reference.columnName
            let query = {}
            for (const key in this.selectedRow) {
                if (this.rowID == this.selectedRow[key]) {
                    query[columnName] =
                        this.selectedRow[this.schema.primary_key_name]
                }
            }

            let location: Location = {
                name: "rowListing",
                params: { tableName: reference.tableName },
                query
            }
            let vueUrl = this.$router.resolve(location).href
            window.open(
                `${document.location.origin}${document.location.pathname}${vueUrl}`,
                "_blank"
            )
        }
    },
    async mounted() {
        await this.fetchTableReferences()
    }
})
</script>

Can you try this changes with reviewer column like this

class Review(Table):
    reviewer = Varchar()
#and then
class Review(Table):
    reviewer = ForeignKey(Director)
# and then
class Review(Table):
    reviewer = ForeignKey(Director, target_column=Director.name)

What do you think about this?

[dantownsend]

@sinisaos It looks promising - I'll give it a go.

[sinisaos]

@dantownsend We also need to make these changes to add and edit records in Piccolo Admin to make everything work.

// changes in EditRowForm.vue
methods: {
    async submitForm(event) {
        console.log("Submitting...")

        const form = new FormData(event.target)

        const json = {}
        const targetColumn = []
        for (const i of form.entries()) {
            const key = i[0].split(" ").join("_")
            let value = i[1]

            if (value == "null") {
                value = null
            } else if (this.schema.properties[key].type == "array") {
                // @ts-ignore
                value = JSON.parse(value)
            } else if (
                this.schema?.properties[key].format == "date-time" &&
                value == ""
            ) {
                value = null
            } else if (
                this.schema?.properties[key].extra.foreign_key == true &&
                value == ""
            ) {
                value = null
            } else if (
                this.schema?.properties[key].extra.foreign_key == true &&
                this.schema?.properties[key].format != "uuid" &&
                this.schema?.properties[key].type != "integer" &&
                key == this.schema?.properties[key].extra.to
            ) {
                targetColumn.push(value)
            }
            json[key] =
                targetColumn[1] == value &&
                this.schema?.properties[key].format != "uuid" &&
                this.schema?.properties[key].type != "integer"
                    ? targetColumn[0]
                    : value
        }
...

and

// changes in AddRowForm.vue
methods: {
    async submitForm(event) {
        console.log("I was pressed")
        const form = new FormData(event.target)

        const json = {}
        const targetColumn = []
        for (const i of form.entries()) {
            const key = i[0].split(" ").join("_")
            let value = i[1]

            if (value == "null") {
                value = null
                // @ts-ignore
            } else if (this.schema.properties[key].type == "array") {
                // @ts-ignore
                value = JSON.parse(value)
                // @ts-ignore
            } else if (
                this.schema?.properties[key].format == "date-time" &&
                value == ""
            ) {
                value = null
                // @ts-ignore
            } else if (
                this.schema?.properties[key].extra.foreign_key == true &&
                value == ""
            ) {
                value = null
            } else if (
                this.schema?.properties[key].extra.foreign_key == true &&
                this.schema?.properties[key].format != "uuid" &&
                this.schema?.properties[key].type != "integer" &&
                key == this.schema?.properties[key].extra.to
            ) {
                targetColumn.push(value)
            }
            json[key] =
                targetColumn[1] == value &&
                this.schema?.properties[key].format != "uuid" &&
                this.schema?.properties[key].type != "integer"
                    ? targetColumn[0]
                    : value
        }
...

[sinisaos]

@dantownsend Did you have time to try this? If you agree with this, I can do PR in Piccolo Admin, with changes from the comments to make everything work.

[dantownsend]

@sinisaos I haven't had a chance, but I would like to get it sorted. If you could create the PR that would be great, thanks.

[sinisaos]

@dantownsend Great. I will wait with PR because these changes in Piccolo Admin are only relevant if the code from this PR is good enough.

[dantownsend]

@sinisaos OK, makes sense

[sinisaos]

@dantownsend Everything should work now. Combination of primary and non-primary keys per table, multiple non-primary keys per table etc.

[sinisaos]

@dantownsend Have you had a chance to try this?

[dantownsend]

@sinisaos Not yet - will try and review it this weekend.

[sinisaos]

@dantownsend Great. Thanks.