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

Birth Records are rejected when trying to sync. #448

Closed
jgaehring opened this issue Sep 8, 2020 · 5 comments
Closed

Birth Records are rejected when trying to sync. #448

jgaehring opened this issue Sep 8, 2020 · 5 comments
Labels

Comments

@jgaehring
Copy link
Member

I've realized that creating birth records in Field Kit results in a broken state, b/c we don't have the UI component for editing the mother field, but we're generating a blank birth record log with mother: { id: null }, which is getting rejected by the server.

The farm_birth resource describes the mother field as follows:

{
  "label": "Mother",
  "type": "entityreference",
  "required": 0,
  "data_schema": {
    "id": null
  }
}

As @mstenta has pointed out, the data_schema is the same as for any field of type entityreference, as defined here:

https://github.com/farmOS/farmOS/blob/da504a1be74f085007f294329c72a368a3029358/modules/farm/farm_ui/farm_ui.module#L58

Yet we're not seeing this issue for, say, the asset field of a log, although, perhaps crucially, the asset field accepts an array of objects, as descibed in the same farm_birth resource:

{
  "label": "Children",
  "type": "entityreference",
  "required": 0,
  "data_schema": [
    {
      "id": null
    }
  ]
}

This is a significant bug, but no urgent, imo. So hopefully this documents the issue thoroughly, and if there's an easy fix, great, but otherwise perhaps it depends on farmOS/farmOS#243.

Oh, and for reference, here's where I originally described how the data_schema field should work: #346 (comment).

@mstenta
Copy link
Member

mstenta commented May 25, 2021

Spoke with @jgaehring about this today - I'm going to move it to the Field Kit issue queue.

It shouldn't be an issue in farmOS/Field Kit 2.x, so we can probably close is as "wontfix" for 1.x. And maybe we can make a 1.x "known issues" list somewhere for reference. But I'll leave it open for now...

@mstenta mstenta transferred this issue from farmOS/farmOS May 25, 2021
@jgaehring
Copy link
Member Author

To be clear, this primarily becomes an issue where the user tries to create a birth record in Field Kit then sync it, whereat the server rejects it, with a 406 IIRC, b/c it requires that mother field. I'm going to rename this so it's a little more obvious for folks who might encounter this issue in the future

Although come to think of it, this could be an issue for 2.x as well, for the very same reason as the equipment issue (#449). TL;DR, we're currently using a stub for JSON Schema evaluation in farmOS.js 2.x, which means we can only use entities' base fields. The mother field on logs is not a base field, so as long as we're using that stub this issue will probably persist for 2.x as well.

I may expand #449 then to address both those issues for 2.x, but will leave this issue dedicated to the 1.x issue, since 1.x will be stuck with these schema issues no matter what and will require a separate workaround, if we choose to fix this at all.

@jgaehring jgaehring changed the title Invalid null value for Birth Record Birth Records are rejected when trying to sync. May 25, 2021
@jgaehring jgaehring added the v1 label May 25, 2021
@mstenta
Copy link
Member

mstenta commented May 25, 2021

Although come to think of it, this could be an issue for 2.x as well ... The mother field on logs is not a base field, so as long as we're using that stub this issue will probably persist for 2.x as well.

Actually this won't be an issue because mother is a base field of birth logs, so that field is guaranteed to exist, and can be hard-coded in Field Kit.

The difference with "Equipment" is that it is added to all logs... and only if the equipment module is enabled.

@jgaehring jgaehring mentioned this issue May 25, 2021
@mstenta
Copy link
Member

mstenta commented May 25, 2021

Also meant to mention: the mother field will not be required on Birth logs in 2.x like they were in 1.x (I think that was sort of an arbitrary limitation in 1.x) - so pushing a birth log without a mother won't cause an error...

@jgaehring
Copy link
Member Author

As with #449, this has been resolved since the schema stubs were replaced (farmOS/farmOS.js#26), along with the fact that the mother field is no longer required, as Mike mentioned in the previous comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants