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.

Sign up for GitHub

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

Implement display options for Duration type #1091

Merged
merged 8 commits into from
Feb 25, 2022
Merged

Implement display options for Duration type #1091

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
766f24e
mathesar-433 Implement display options for Duration type
silentninja Feb 21, 2022
74f2c40
Merge branch 'master' into mathesar-433-duration-display-options
silentninja Feb 21, 2022
683847d
Add `Duration` type display options to types list
silentninja Feb 22, 2022
6c22eaa
Merge remote-tracking branch 'origin/mathesar-433-duration-display-op…
silentninja Feb 22, 2022
4a9f1e0
Merge branch 'master' into mathesar-433-duration-display-options
silentninja Feb 23, 2022
f7692f3
Fix duration display options to accept timestamp display options inst…
silentninja Feb 23, 2022
9b7acc8
Merge remote-tracking branch 'origin/mathesar-433-duration-display-op…
silentninja Feb 23, 2022
e124ca2
Merge branch 'master' into mathesar-433-duration-display-options
silentninja Feb 24, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions mathesar/api/display_options.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@
{"name": "locale", "type": "string"}]
},
MathesarTypeIdentifier.DATETIME.value:
{
"options": [{"name": "format", "type": "string"}]
},
MathesarTypeIdentifier.DURATION.value:
{
"options": [{"name": "format", "type": "string"}]
}
Expand Down
15 changes: 14 additions & 1 deletion mathesar/api/serializers/shared_serializers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ def validate(self, datetime_obj, display_format, serializer_field):
class TimeWithTimeZoneFormatValidator(AbstractDateTimeFormatValidator):

def validate(self, datetime_obj, display_format, serializer_field):
time_only_format = 'HH:mm:ssZZ'
time_only_format = 'HH:mm:ss.SSSZZ'
time_str = arrow.get('2013-09-30T15:34:00.000-07:00').format(time_only_format)
parsed_time_str = arrow.get(time_str, time_only_format)
if parsed_time_str.date() != datetime_obj.date():
Expand All @@ -180,6 +180,14 @@ def validate(self, datetime_obj, display_format, serializer_field):
return super().validate(datetime_obj, display_format, serializer_field)


class DurationFormatValidator(TimeWithoutTimeZoneFormatValidator):
"""
The validation is same as Time Without TimeZone as of now,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not make sense. Durations can span weeks, months, days, and years. Times cannot.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed it, it was meant to be timestamp. I have changed the validation method anyway since we need a different error message, though the logic is similar.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I haven't restricted using a weekdate format string(2019-W17), though the duration value returned from the database follows RFC 3339 and does not return a weekdate, as the user might have a different requirement

it has been subclassed without any overrides so that the validation logic can be changed in future if needed
"""
pass


class DateDisplayOptionSerializer(MathesarErrorMessageMixin, OverrideRootPartialMixin, serializers.Serializer):
format = serializers.CharField(validators=[DateFormatValidator()])

Expand Down Expand Up @@ -216,6 +224,10 @@ class TimeWithoutTimezoneDisplayOptionSerializer(
format = serializers.CharField(validators=[TimeWithoutTimeZoneFormatValidator()])


class DurationDisplayOptionSerializer(MathesarErrorMessageMixin, OverrideRootPartialMixin, serializers.Serializer):
format = serializers.CharField(validators=[DurationFormatValidator()])


class DisplayOptionsMappingSerializer(
MathesarErrorMessageMixin,
ReadWritePolymorphicSerializerMappingMixin,
Expand All @@ -231,6 +243,7 @@ class DisplayOptionsMappingSerializer(
('date', MathesarTypeIdentifier.DATETIME.value): DateDisplayOptionSerializer,
('time with time zone', MathesarTypeIdentifier.DATETIME.value): TimeWithTimezoneDisplayOptionSerializer,
('time without time zone', MathesarTypeIdentifier.DATETIME.value): TimeWithoutTimezoneDisplayOptionSerializer,
MathesarTypeIdentifier.DURATION.value: DurationDisplayOptionSerializer,
}

def get_mapping_field(self):
Expand Down
2 changes: 2 additions & 0 deletions mathesar/tests/api/test_column_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ def test_column_create_invalid_default(column_test_table, client):
("BOOLEAN", {"input": "dropdown"}),
("BOOLEAN", {"input": "checkbox", "custom_labels": {"TRUE": "yes", "FALSE": "no"}}),
("DATE", {'format': 'YYYY-MM-DD'}),
("INTERVAL", {'format': 'HH:mm:ss.SSS'}),
("NUMERIC", {"show_as_percentage": True}),
("NUMERIC", {"show_as_percentage": True, "locale": "en_US"}),
("TIMESTAMP WITH TIME ZONE", {'format': 'YYYY-MM-DD hh:mm'}),
Expand All @@ -256,6 +257,7 @@ def test_column_create_display_options(
name = "anewcolumn"
data = {"name": name, "type": type_, "display_options": display_options}
response = client.post(f"/api/db/v0/tables/{column_test_table.id}/columns/", data)
print(response.data)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this print statement.

assert response.status_code == 201

# Ensure the correct serialized date is returned by the API
Expand Down