Add acceptance test for correct format on date fields

[timroes]

We currently don't set the format property on all type: string fields in connectors that accept a date. The format property has currently no effect, but will be once we have date pickers in the UI, since it's the one determining if a string field is actually a date.

We should add an acceptance test, that tests that:

a) if a type: string field has the pattern ^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}Z$, that it must also have a format: date-time
b) if a type: string field has the pattern ^[0-9]{4}-[0-9]{2}-[0-9]{2}$, it must also have format: date

[erohmensing]

@timroes as far as I understand, the current state is:

• When a field should be a date, it's stored as a string
• In order to help users input the correct format for this string, we define a pattern in the spec, and use that regular expression to validate on the frontend
• The format field is available to use in the spec and readable/usable from the frontend
• We may or may not (?) already set the format field either manually or injected in the frontend (this one needs clarifying)

I think you're asking for:

• If we read that a string field has a pattern that we (somewhat arbitrarily) think is a date, SAT should fail and specify that that connector needs a format: <date/date-time> field

What I think you're trying to get to is "If something should explicitly be a date, it needs to be marked with the format: date field" so that we can use the date-picker. Makes sense to me but I don't think we need to look at the regexes for that, we need to update the existing connectors and the existing documentation. As far as I can tell, the regexes are only in place because we hadn't implemented the datepicker yet! Being explicit about definining what's a date can avoid accidentally pulling up the date picker for a field that only looks (via regex) like a date.

Here's what I'd suggest:

• Introduce format: date and format: date-time if frontend needs to distinguish these two for date picker and date-time picker
• Optional: If we need to dump the date from the frontend into a specific format, introduce a field where we let the connector specify which date format it likes (e.g. YYYY-MM-DD out of a list of ones we support/can dump from the frontend based on whatever library we do to do the dumping). If this were the case we probably don't need both date and date-time.
• Wherever our current connectors use the pattern to indicate dates currently, manually update those to use one of the formats instead, removing the pattern.
• Update our docs with a validation section under "Actor Specification" which lists more clearly what type of validation is used in the frontend (e.g. patterns and numbers can validate your input, enum creates a dropdown(?), etc. Related but out of scope: How many of these other [validation keywords](https://json-schema.org/draft/2020-12/json-schema-validation.html#name-enum](https://json-schema.org/draft/2020-12/json-schema-validation.html#name-a-vocabulary-for-structural) are supported by our frontend validation?

[timroes]

We may or may not (?) already set the format field either manually or injected in the frontend (this one needs clarifying)

There is nothing in the frontend that would "set" the format. We'll use it later to define if a field requires a date picker in the UI, since the official definition in JSONSchema is that a date field is type: string, format: date(-time).

Makes sense to me but I don't think we need to look at the regexes for that, we need to update the existing connectors and the existing documentation.

My idea was that it seems those fields are sometimes just missed by accident (no matter how good the documentatin), so having the SAT is just a layer to make sure, those are not forgotten (same as e.g. other SAT tests for duplicate enum values).

Being explicit about definining what's a date can avoid accidentally pulling up the date picker for a field that only looks (via regex) like a date.

I don't see where this could be the case (at least for format: date-time)? If a field expects a pattern that's exactly an ISO date string, I don't see what would be the reason the user shouldn't select that via a datepicker? Do you maybe have an example where we'd ever have the use-case that a field is a strnig, exactly looking like a date, but not actually being a date?

[timroes]

Optional: If we need to dump the date from the frontend into a specific format, introduce a field where we let the connector specify which date format it likes (e.g. YYYY-MM-DD out of a list of ones we support/can dump from the frontend based on whatever library we do to do the dumping). If this were the case we probably don't need both date and date-time.

I'm not sure I follow this to be honest. The format value of date or date-time is already what defines the actual format, since JSONSchema is expecting spcific formats: https://json-schema.org/understanding-json-schema/reference/string.html#dates-and-times. We can't have a date field and then decide we'd like the format though to be YYYY/DD/MM but still use format: date.

[timroes]

Update our docs with a validation section under "Actor Specification" which lists more clearly what type of validation is used in the frontend (e.g. patterns and numbers can validate your input, enum creates a dropdown(?), etc. Related but out of scope: How many of these other [validation keywords](https://json-schema.org/draft/2020-12/json-schema-validation.html#name-enum](https://json-schema.org/draft/2020-12/json-schema-validation.html#name-enum%5D(https://json-schema.org/draft/2020-12/json-schema-validation.html#name-a-vocabulary-for-structural)) are supported by our frontend validation?

We unfortunately don't have that documented at the moment, which JSONSchema values are actually supported in the UI anywhere explicitally. I think it would be a valid task to document the exact supported features somewhere, to make it easier to see. (cc @lmossman) That said, we should ideally use in the spec whatever JSONSchema makes sense, and if the UI doesn't support it yet, we should build that support.

[erohmensing]

We want to use format to indicate that the UI should pull up a datepicker

Makes sense 👍🏻

the official definition in JSONSchema is that a date field is type: string, format: date(-time)

Cool, I had no idea! This seems like something else we can enforce ourselves, we just need to document that. I was looking at the metaschema, which doesn't include it, because the validation doc has it as a SHOULD and therefore doesn't enforce it (like the enum values).

My idea was that it seems those fields are sometimes just missed by accident (no matter how good the documentatin)
I don't see where this could be the case (at least for format: date-time)

If the property is already annotated with format: date or format: datetime I definitely agree. However for just a given text field with a certain regex, I don't think we should default to datepicker. If some source has a post ID (for example) that's of format nn-nn-nnnn, but is just some numbers, I feel like it'd be very strange to have to use a datepicker to enter that ID?

However I do think it makes sense to, when we start using format, look at which connectors already use date regexes to migrate them over. Perhaps we could make something that doesn't fail, but alerts the connector writer that the property they have looks like it could be a date, here's the info about how to correctly indicate that with format: date

[erohmensing]

We unfortunately don't have that documented at the moment, which JSONSchema values are actually supported in the UI anywhere explicitally. I think it would be a valid task to document the exact supported features somewhere, to make it easier to see. (cc @lmossman) That said, we should ideally use in the spec whatever JSONSchema makes sense, and if the UI doesn't support it yet, we should build that support.

I agree, I'd be happy to make a more general issue collaborating with your team on it - I mentioned it in my PR here

[lmossman]

@erohmensing If you could create an issue to capture which information is needed from the frontend, with the labels team/extensibility and team/frontend, that would be awesome! And to help gauge the priority on this: is this lack of documentation causing continuous confusion for your team, or is this more of a nice to have?

[erohmensing]

Definitely a nice to have, low prio :) more of a question of catching things we want to enforce before we run into them -- so understanding what should be enforced based on the expected frontend experience

For example: lists shouldn't have duplicates; if a list has a default value, the list also needs to have a "default" option (or maybe that is something that the frontend should infer via there being a default value, and adding that option instead? multiple ways to do something, but looking at those rules makes us sure that we've made conscious decisions about them)

[erohmensing]

@timroes I'd propose closing this in favor of airbytehq/airbyte-internal-issues#1129 where we would define all of the restrictions we want to SAT test. Sound good?

[erohmensing]

If the goal is to mark any properties that have those regexes as date or date-time so that they can be used in the datepicker, I still think that can be done pretty easily (perhaps by the GL team that owns the connectors?) but I think at least for the date format, the expected string format and associated regex is too generic for one to always expect it to be a date. You could probably convince me for date-time though 😄