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

Unify datetime format #1389

Merged
merged 6 commits into from
Jul 4, 2023
Merged

Unify datetime format #1389

merged 6 commits into from
Jul 4, 2023

Conversation

PiTrem
Copy link
Member

@PiTrem PiTrem commented Jul 3, 2023

  • use formatDate utility to format displayed timestamps
    (reduce the number of 'import moment.js' )

  • Entity: return time zone in timestamp

  • more entities are now using the same exposure (expose_timestamp) for timestamps

  • shoudl resolve timezone diff in comment-feature and notification-feature

@@ -5,6 +5,7 @@ class ApplicationEntity < Grape::Entity
CUSTOM_ENTITY_OPTIONS = %i[anonymize_below anonymize_with].freeze

format_with(:eln_timestamp) do |datetime|
#datetime.present? ? I18n.l(datetime, format: :eln_iso8601) : nil
Copy link

Choose a reason for hiding this comment

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

Layout/LeadingCommentSpace: Missing space after #.

@PiTrem PiTrem force-pushed the unify-datetime-format branch 2 times, most recently from b6ec01d to b6037ab Compare July 3, 2023 15:21
@PiTrem PiTrem mentioned this pull request Jul 4, 2023
@@ -20,5 +20,13 @@ class DeviceMetadataEntity < Grape::Entity
expose :data_cite_created_at, documentation: { type: 'DateTime', desc: 'created_at DataCite ' }
expose :data_cite_updated_at, documentation: { type: 'DateTime', desc: 'updated_at DataCite' }
expose :data_cite_version, documentation: { type: 'Integer', desc: 'version at DataCite' }

def data_cite_created_at
object.data_cite_created_at.present? ? && I18n.l(object.data_cite_created_at, format: :eln_timestamp) : nil
Copy link

Choose a reason for hiding this comment

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

Lint/Syntax: unexpected token tAMPER
(Using Ruby 2.7 parser; configure using TargetRubyVersion parameter, under AllCops)

@@ -20,5 +20,13 @@ class DeviceMetadataEntity < Grape::Entity
expose :data_cite_created_at, documentation: { type: 'DateTime', desc: 'created_at DataCite ' }
expose :data_cite_updated_at, documentation: { type: 'DateTime', desc: 'updated_at DataCite' }
expose :data_cite_version, documentation: { type: 'Integer', desc: 'version at DataCite' }

def data_cite_created_at
object.data_cite_created_at.present? ? && I18n.l(object.data_cite_created_at, format: :eln_timestamp) : nil
Copy link

Choose a reason for hiding this comment

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

Lint/Syntax: unexpected token tCOLON
(Using Ruby 2.7 parser; configure using TargetRubyVersion parameter, under AllCops)

end

def data_cite_updated_at
object.data_cite_updated_at.present? ? && I18n.l(object.data_cite_updated_at, format: :eln_timestamp) : nil
Copy link

Choose a reason for hiding this comment

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

Lint/Syntax: unexpected token tAMPER
(Using Ruby 2.7 parser; configure using TargetRubyVersion parameter, under AllCops)

end

def data_cite_updated_at
object.data_cite_updated_at.present? ? && I18n.l(object.data_cite_updated_at, format: :eln_timestamp) : nil
Copy link

Choose a reason for hiding this comment

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

Lint/Syntax: unexpected token tCOLON
(Using Ruby 2.7 parser; configure using TargetRubyVersion parameter, under AllCops)

@github-actions
Copy link

github-actions bot commented Jul 4, 2023

LCOV of commit f176352 during Continuous Integration #1231

Summary coverage rate:
  lines......: 59.4% (12269 of 20648 lines)
  functions..: no data found
  branches...: no data found

Files changed coverage rate: n/a

Copy link
Contributor

@FabianMauz FabianMauz left a comment

Choose a reason for hiding this comment

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

Looks good

Copy link
Contributor

Choose a reason for hiding this comment

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

checked if response of endpoint is not used any more. Can confirm it.

It is harvested by MessagesFetcher.createMessage but then in further processing thrown away:

MessagePublish.js line 51
UserManagement.js line 453

@github-actions
Copy link

github-actions bot commented Jul 4, 2023

LCOV of commit 7ac1cc0 during Continuous Integration #1234

Summary coverage rate:
  lines......: 59.4% (12270 of 20647 lines)
  functions..: no data found
  branches...: no data found

Files changed coverage rate: n/a

@github-actions
Copy link

github-actions bot commented Jul 4, 2023

LCOV of commit ffb5661 during Continuous Integration #1237

Summary coverage rate:
  lines......: 59.4% (12269 of 20648 lines)
  functions..: no data found
  branches...: no data found

Files changed coverage rate: n/a

@PiTrem PiTrem merged commit fb471bd into main Jul 4, 2023
3 checks passed
mekkyz pushed a commit that referenced this pull request Sep 21, 2023
* use formatDate utility to format displayed timestamps
(reduce the number of 'import moment.js' )

* fix Intl time format

* Entities: use default timestamp formatter

* timezoneHelper: default to iso format if parsed value invalid.

Also use short format date llll, not LLLL.

TODO use locale ?
baolanlequang pushed a commit that referenced this pull request Mar 5, 2024
* use formatDate utility to format displayed timestamps
(reduce the number of 'import moment.js' )

* fix Intl time format

* Entities: use default timestamp formatter

* timezoneHelper: default to iso format if parsed value invalid.

Also use short format date llll, not LLLL.

TODO use locale ?
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.

None yet

2 participants