search

thunderbird / thunderbird-notifications

In-App notifications for Thunderbird
Mozilla Public License 2.0
0 stars 0 forks source link

Add draft of schema file #2

Closed radishmouse closed 4 months ago

radishmouse commented 6 months ago

Closes #1

The new schema.json includes the fields mentioned in the tech spec as well as:

Please let me know if anything is missing or needs improvement!

Update: now includes a version of the schema generated by the pydantic model (from #5). With the exception of the dependantRequired key , it is equivalent to the schema.json introduced by this PR.

MelissaAutumn commented 6 months ago

cc @freaktechnik & @vDeo

Do you two have any questions, or concerns about this spec?

radishmouse commented 6 months ago

I've got a more fields to add (thankfully, @Sancus alerted me to the missing fields), but won't be able to address until tomorrow

radishmouse commented 6 months ago

.

I just had a thought: we can handle localization with the locale filtering, so we have one of these objects per locale and just filter them.

@MelissaAutumn suggested something similar: maybe when the client requests the notifications, it sends a locale string as a parameter. That way, the server only sends back the locale-specific notifications.

freaktechnik commented 6 months ago

Mel suggested something similar: maybe when the client requests the notifications, it sends a locale string as a parameter. That way, the server only sends back the locale-specific notifications.

Right, but that would mean you're doing actual work on the server side. As I had mentioned, we should easily be able to do an URL similar to what the release notes have: https://searchfox.org/comm-central/rev/c04464d92274c9d2b8ec036d47b0c664adf1a45f/mail/app/profile/all-thunderbird.js#127

radishmouse commented 5 months ago

@freaktechnik @Sancus The changes introduced by #5 include a command for generating a JSON schema from a pydantic model.

The pydantic model includes all the same enum values and constraints as the JSON schema in this PR.

Would it be reasonable to close this PR in favor of a generated schema file? Alternatively, we could create another ticket to test the generated JSON schema more thoroughly.

MelissaAutumn commented 5 months ago

@freaktechnik @Sancus The changes introduced by #5 include a command for generating a JSON schema from a pydantic model.

The pydantic model includes all the same enum values and constraints as the JSON schema in this PR.

Would it be reasonable to close this PR in favor of a generated schema file? Alternatively, we could create another ticket to test the generated JSON schema more thoroughly.

Hey @radishmouse can you commit a current copy of that schema to repo for reference? And a general note it is missing the dependantRequired key (https://github.com/thunderbird/thunderbird-notifications/issues/7) but is otherwise matching.

radishmouse commented 5 months ago

@MelissaAutumn generated schema added to this PR

Also, I updated the PR description making it clear that the generated schema is missing the dependantRequired key (but otherwise matches).

freaktechnik commented 5 months ago

I assume all the optional keys being nullable is a side effect of what it's being generated from?

I think the manually written schema currently captures some less formal conventions on the interface, which the generated schema doesn't document.

radishmouse commented 4 months ago

It sounds like the manually written schema is preferred. I'll merge this PR and update the JSON validation GH action to use it.