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

Extract display date and time methods to a helper #3572

Merged
merged 1 commit into from
Feb 25, 2025

Conversation

minhngocd
Copy link
Contributor

There are duplicate code where display date is used (e.g. consultation and call for evidence). We also currently have a hard set reliance on the content item presenter to use date displays. Extracting this to a helper module will allow us to remove that coupling.

Additionally, there are stubs of the method all over tests which means tests are not testing actual values. Have removed some of this stubbing.

⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

Follow these steps if you are doing a Rails upgrade.

Routes in this application are being migrated to another application, please check with #govuk-patterns-and-pages when making changes.

This is to remove duplication and also prepare for model creations so we can reuse these methods without having to rely on the ContentItem module.
This also allows us to remove the stubs placed in tests and test actual values passed through.
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-3572 February 24, 2025 17:57 Inactive
Copy link
Contributor

@hannako hannako left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -35,13 +35,9 @@ def withdrawal_notice_context

def withdrawal_notice_time
view_context.tag.time(
english_display_date(withdrawal_notice["withdrawn_at"]),
DateTimeHelper.display_date(withdrawal_notice["withdrawn_at"], locale: :en),
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh this is a pain isn't it! I'll make a card on the navigation backlog to log that we need a translation for

def withdrawal_notice_title
"This #{withdrawal_notice_context.downcase} was withdrawn on #{withdrawal_notice_time}".html_safe
end

@minhngocd minhngocd merged commit 67d5934 into main Feb 25, 2025
12 checks passed
@minhngocd minhngocd deleted the extract-date-time-helper branch February 25, 2025 15:20
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.

3 participants