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

Split Date & Time data type into three Mathesar types #960

Closed
Tracked by #250
kgodey opened this issue Jan 7, 2022 · 8 comments · Fixed by #1093
Closed
Tracked by #250

Split Date & Time data type into three Mathesar types #960

kgodey opened this issue Jan 7, 2022 · 8 comments · Fixed by #1093
Assignees
Labels
restricted: maintainers Only maintainers can resolve this issue type: enhancement New feature or request work: backend Related to Python, Django, and simple SQL

Comments

@kgodey
Copy link
Contributor

kgodey commented Jan 7, 2022

Problem

Based on the new definition of Mathesar Data Types, it makes sense to split up the Date & Time type into three types:

  • Date
  • Time
  • Date & Time

Proposed solution

We need to update the data type design specs to create specs for Date and Time and update the Date & Time specs to remove the options for date-only and time-only database options.

Additional context

@kgodey kgodey added type: enhancement New feature or request work: design ready Ready for implementation labels Jan 7, 2022
@kgodey kgodey added this to the [07] Initial Data Types milestone Jan 7, 2022
@silentninja
Copy link
Contributor

I think Date and Date & Time can be under the same Mathesar type similar to FLOAT and INTEGER. The Time type seems like an odd one as the cast with another type will change the meaning drastically

@kgodey
Copy link
Contributor Author

kgodey commented Jan 8, 2022

I think it will be confusing for Date to be in Date & Time but not Time, it's inconsistent.

@ghislaineguerin ghislaineguerin added status: started and removed ready Ready for implementation labels Jan 13, 2022
@kgodey kgodey changed the title Split Date & Time data type into three types Split Date & Time data type into three Mathesar types Feb 4, 2022
@kgodey kgodey added restricted: maintainers Only maintainers can resolve this issue work: backend Related to Python, Django, and simple SQL ready Ready for implementation and removed restricted: design team labels Feb 4, 2022
@kgodey
Copy link
Contributor Author

kgodey commented Feb 4, 2022

Now that the design for this is done, we need to make sure the backend reflects the new types correctly. We should make sure the Mathesar Types API, display options, etc. all work correctly based on the new design.

@dmos62
Copy link
Contributor

dmos62 commented Feb 22, 2022

@pavish @seancolsen currently the display options for the datetime UI/Mathesar type look like this (excerpt from UI types endpoint):

  {
    "identifier": "datetime",
    "name": "Date & Time",
    "db_types": [
      "DATE",
      "TIME WITH TIME ZONE",
      "TIME WITHOUT TIME ZONE",
      "TIMESTAMP WITH TIME ZONE",
      "TIMESTAMP WITHOUT TIME ZONE"
    ],
    "filters": null,
    "display_options": {
      "options": [
        {
          "name": "format",
          "type": "string"
        }
      ]
    }
  },

What should the display options for the upcoming date, time and datetime UI types be? Should they be the same? I.e.:

    "display_options": {
      "options": [
        {
          "name": "format",
          "type": "string"
        }
      ]
    }

@kgodey
Copy link
Contributor Author

kgodey commented Feb 22, 2022

@dmos62 Please consult the display options in the Jan 2022 date time prototype in the Global Date Time Components design spec. It seems like we need either a date_format or a time_format option, or both, depending on the type.

@dmos62
Copy link
Contributor

dmos62 commented Feb 22, 2022

@kgodey I'm still wrapping my head around display options and their current state. I guess date_format and time_format can be boiled down to just format, like it is for datetime currently. So, date and time types would have the same display options declared in this endpoint as datetime does now: {"name": "format","type": "string"}. Does that make sense?

I see @mathemancer submitted a PR adding this same display option (format) to the duration type, which seems to confirm that that's enough.

@kgodey
Copy link
Contributor Author

kgodey commented Feb 22, 2022

@dmos62 I think we need separate date and time formats for Date & Time (so that the user can set separate formats for the date and time component). Dates would just use the date format and Times would use the time format. So that means:

  • Date & Time would have both a date_format and a time_format
  • Date would have a date_format
  • Time would have a time_format

The single format attribute came from #428, but that ticket was written before the new design specs. As you can see in the design, there are two separate options for the frontend to set for date format and time format for Date & Time types. The display_options attribute is for the frontend to save both those options.

Durations are represented differently and the format for durations shouldn't affect this issue.

@dmos62
Copy link
Contributor

dmos62 commented Feb 22, 2022

@kgodey oh, my monitor is in profile and the datetime prototype was out of view so I couldn't see it until I started panning.

Ok, I'll create date_format and time_format display options.

@kgodey kgodey removed the ready Ready for implementation label Mar 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
restricted: maintainers Only maintainers can resolve this issue type: enhancement New feature or request work: backend Related to Python, Django, and simple SQL
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

4 participants