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

Portuguese Date Fixed .pm #624

Merged
merged 3 commits into from
Jan 10, 2025
Merged

Portuguese Date Fixed .pm #624

merged 3 commits into from
Jan 10, 2025

Conversation

Bashizz
Copy link
Contributor

@Bashizz Bashizz commented Jan 6, 2025

Proposed change

There are some users that like to put the language in Portuguese from Portugal, the problem was that the date appeared as "DateFormatInvalid" due to the date not being applied correctly (not applied at all), so I basically just copied the brazilian one and pasted it over the already existing code

🐞 bug

Breaking change

It doesn't break

Additional information

Checklist

  • The code change is tested and works locally.(❗)
  • There is no commented out code in this PR.(❕)
  • You improved or added new unit tests.(❕)
  • Local ZnunyCodePolicy passed.(❕)
  • Local UnitTests / Selenium passed.(❕)
  • GitHub workflow CI (UnitTests / Selenium) passed.(❗)

Portuguese Date Format
@rkaldung rkaldung added the 4 - verified This issue or pull request was verified. label Jan 7, 2025
@rkaldung
Copy link
Member

rkaldung commented Jan 7, 2025

Verified with public sources:

  • Mostly (dd/mm/yyyy) and (dd-mm-yyyy); some newer documents use (yyyy-mm-dd)
  • Standard described in NP EN 28601

@rkaldung rkaldung assigned rkaldung and mbethke and unassigned rkaldung Jan 7, 2025
@rkaldung
Copy link
Member

rkaldung commented Jan 7, 2025

@mbethke Please check if this applies to both current versions

@dennykorsukewitz
Copy link
Member

@mbethke
Copy link

mbethke commented Jan 7, 2025

@dennykorsukewitz I'm not entirely sure what state to use here. @Bashizz' change certainly fixes the bug in that it now works ;) But it uses the Brazilian format which seems to be neither the most common in Portugal nor conforming to ISO-8601.
The translators at the EU commission noted (already 20 years ago, but it's the latest I could find) that NP EN 28601 just recommends the ISO format %Y-%M-%D but it would only be a good idea for something like a form or invoice, not necessarily for texts closer to the oral form where they'd prefer what we had before, %D de %B de %Y.
Znuny translations are all over the place on whether DateFormatLong should include a weekday name—21 languages don't, 27 do. I tend towards "do".
Looking at the other translation files I also noticed that both zh_TW and zh_CN still have the leading space in DateFormatLong, and Korean has the same problems as non-Brazilian Portuguese, so some more comprehensive fixing is needed.

Proposal for pt.pm, basically what we had before but dropping the uncommon hyphen before %T in DateInputFormatLong:

    $Self->{DateFormat}          = '%Y-%M-%D %T';
    $Self->{DateFormatLong}      = '%A, %D de %B de %Y, %T';
    $Self->{DateFormatShort}     = '%Y-%M-%D';
    $Self->{DateInputFormat}     = '%Y-%M-%D';
    $Self->{DateInputFormatLong} = '%Y-%M-%D %T';

@rkaldung there hadn't been any changes to DateFormat between the rel-6_5 branch and the before the above commit from September so it will be fine to apply fixes to both 6.5.x and 7.1.

@mbethke
Copy link

mbethke commented Jan 8, 2025

Internal issue is 1007

@mbethke mbethke added 1 - 🐞 bug 🐞 An issue with the system. 3 - wait for reviewer Znuny, it's your turn. labels Jan 8, 2025
Copy link
Member

@dennykorsukewitz dennykorsukewitz left a comment

Choose a reason for hiding this comment

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

Hi @Bashizz ,
thanks for your PR.

Approved 👍

We have created an issue internally and will adapt the format a little.

Thank you very much, keep up the good work
Regards 🚀

@dennykorsukewitz dennykorsukewitz added this to the rel-7_1_4 milestone Jan 10, 2025
@dennykorsukewitz dennykorsukewitz added 3 - wait for merge Znuny, it's your turn. and removed 3 - wait for reviewer Znuny, it's your turn. labels Jan 10, 2025
@dennykorsukewitz dennykorsukewitz merged commit 3b90ea8 into znuny:dev Jan 10, 2025
13 of 14 checks passed
@dennykorsukewitz dennykorsukewitz removed the 3 - wait for merge Znuny, it's your turn. label Jan 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 - 🐞 bug 🐞 An issue with the system. 4 - verified This issue or pull request was verified.
Development

Successfully merging this pull request may close these issues.

4 participants