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

zone: make Owner fields optional #232

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

thomasqueirozb
Copy link

@thomasqueirozb thomasqueirozb commented Jan 19, 2024

According to the docs id and name are not required and email isn't even mentioned. My request had id and email as null, which broke deserialization

      "owner": { "id": null, "type": "user", "email": null },

The correct thing to do would be to make these fields Option<String> but I didn't do so in order not to break the api. Had to convert the fields to Option, which breaks the public API, but I doubt anyone is using the Owner struct tbh.

This is also how the go implementation does it too:

https://github.com/cloudflare/cloudflare-go/blob/325729d2e358cd3f300c130cfa828e5704e725d8/zone.go#L24-L29

If any of the fields are null, the go json library will just make that field an empty string.

@thomasqueirozb thomasqueirozb changed the title zone: make Owner have default values zone: make Owner fields optional Jan 21, 2024
kennetheversole pushed a commit that referenced this pull request Jan 30, 2025
Does the following:
- Removes the `multiple_railguns_allowed` field, as it appears to be
removed. Fixes #236
- Makes the fields of the `owner` field optional (#232), as otherwise
the examples would still not work
 - Adds the `activated_on` field

There are some more discrepancies, i.e. fields on the docs that don't
exist here, and fields here that don't exist in the docs, but I want to
refrain from changing too much.



This can be tested with the `cloudflare-examples` cli:

<details>
<summary>Before</summary>

```
$ cargo run --bin cloudflare-examples -- --auth-token=XXX zone XXX
   Compiling cloudflare v0.12.0 (/home/wp/programming/cloudflare-rs/cloudflare)
   Compiling cloudflare-examples v0.6.0 (/home/wp/programming/cloudflare-rs/cloudflare-examples)
    Finished dev [unoptimized + debuginfo] target(s) in 3.45s
     Running `target/debug/cloudflare-examples --auth-token=XXX zone XXX`
Error: error decoding response body: missing field `multiple_railguns_allowed` at line 1 column 585
```
</details>

<details>
<summary>After</summary>

```
$cargo run --bin cloudflare-examples -- --auth-token=XXX zone XXX
    Finished dev [unoptimized + debuginfo] target(s) in 0.11s
     Running `target/debug/cloudflare-examples --auth-token=XXX zone XXX`
Success: ApiSuccess {
    result: Zone {
        id: "XXX",
        name: "XXX",
        account: AccountDetails {
            id: "XXX",
            name: "XXX",
        },
        activated_on: 2021-08-22T18:52:08.877823Z,
        betas: None,
        created_on: 2021-08-22T18:48:11.312863Z,
        deactivation_reason: None,
        development_mode: 0,
        host: None,
        meta: Meta {
            custom_certificate_quota: 0,
            page_rule_quota: 3,
            phishing_detected: false,
        },
        modified_on: 2021-08-22T18:52:08.877823Z,
        name_servers: [
            "pedro.ns.cloudflare.com",
            "surina.ns.cloudflare.com",
        ],
        original_dnshost: None,
        original_name_servers: Some(
            [
                "ns13.domaincontrol.com",
                "ns14.domaincontrol.com",
            ],
        ),
        original_registrar: Some(
            "namecheap, inc. (id: 1068)",
        ),
        owner: User {
            id: None,
            email: None,
        },
        paused: false,
        permissions: [
            "#dns_records:edit",
            "#dns_records:read",
            "#zone:read",
        ],
        plan: Some(
            Plan {
                id: "0feeeeeeeeeeeeeeeeeeeeeeeeeeeeee",
                name: "Free Website",
                price: 0.0,
                currency: "USD",
                frequency: Some(
                    Nil,
                ),
                legacy_id: "free",
                is_subscribed: false,
                can_subscribe: false,
            },
        ),
        plan_pending: None,
        status: Active,
        vanity_name_servers: None,
        zone_type: Full,
    },
    result_info: None,
    messages: Array [],
    errors: [],
}
```
</details>
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.

1 participant