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

Jump to bottom

1843: Missing application labels #1855

Merged
merged 4 commits into from
Jan 14, 2025
Merged

1843: Missing application labels #1855

merged 4 commits into from
Jan 14, 2025

Conversation

f1sh1918
Copy link
Contributor

@f1sh1918 f1sh1918 commented Jan 8, 2025 •
edited
Loading

Short description

Some application labels were not resolved properly

Proposed changes

  • rename i18n keys according to how they will stored in the database to be able to resolve them properly,
  • adjust these keys in the form states where they will be set
  • fix key of goldenCardHonoredByMinisterPresidentEntitlement,
  • consider parents for resolving keys for attachments,
  • fix wrong label (Typo in the online application form- Bavaria #1852)

Side effects

  • Maybe some attachment labels may be affected so please check all labels of an application card properly

Testing

  1. Create applications with the different requirements, all possible for the blue card and all possible for the golden card
  2. Open the application list and check if all application labels were resolved correctly, due to the de.json, there were some keys like certificate resolved wrong because the parents wasn't used

Resolved issues

Fixes: #1843
Fixes: #1852

@f1sh1918
1843: add missing requirement labels, fix naming of goldenCardHonored…
0b63941 
…ByMinisterPresidentEntitlement, consider parents for resolving keys for attachments, fix wrong label
michael-markl
michael-markl approved these changes Jan 8, 2025
View reviewed changes
Copy link
Member

@michael-markl michael-markl left a comment

Choose a reason for hiding this comment

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

The architecture of these translations keys seems quite fragile to me...

I haven't checked if you caught every translation key. I also did not test anything.

administration/src/bp-modules/applications/JsonFieldView.tsx Outdated Show resolved Hide resolved
@f1sh1918 f1sh1918 force-pushed the 1843-missing-application-labels branch from fa961e2 to 55f5d48 Compare January 9, 2025 09:47
@f1sh1918 f1sh1918 requested a review from michael-markl January 9, 2025 09:48
@f1sh1918
Copy link
Contributor Author

f1sh1918 commented Jan 9, 2025 •
edited
Loading

@seluianova and @bahaaTuffaha this pr needs really proper testing, since we will have bad issues with wrong i18n keys in the database, if there is a mess.
So i suggest to create all types of applications /golden and blue with all possible requirements options before checking out this pr.
Then checkout this branch and create all types of applications again to check if old applications work and new applications (resolve all i18n labels)

If you have any questions don't hesitate to ask me

@seluianova seluianova self-assigned this Jan 13, 2025
@seluianova
Copy link
Contributor

Duplicated ":"
image

seluianova
seluianova approved these changes Jan 13, 2025
View reviewed changes
Copy link
Contributor

@seluianova seluianova left a comment

Choose a reason for hiding this comment

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

I checked every application type (or I hope so)
Just a couple nitpicks from my side 👍

seluianova
seluianova reviewed Jan 13, 2025
View reviewed changes
administration/src/util/translations/de.json
@@ -20,11 +20,25 @@
"wantsDigitalCard": "Ich beantrage eine digitale Ehrenamtskarte",
"wantsPhysicalCard": "Ich beantrage eine physische Ehrenamtskarte",
"blueCardJuleicaEntitlement": "Ich bin Inhaber:in einer JuLeiCa (Jugendleiter:in-Card)",
"blueCardWorkAtDepartmentEntitlement": "Ich bin aktiv in der Freiwilligen Feuerwehr mit abgeschlossener Truppmannausbildung bzw. abgeschlossenem Basis-Modul der Modularen Truppausbildung (MTA), oder im Katastrophenschutz oder im Rettungsdienst mit abgeschlossener Grundausbildung.",
Copy link
Contributor

Choose a reason for hiding this comment

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

🙃 maybe we don't need the "." at the end of the entitlement descriptions.
or at least it should be the same for all items.
currently there is no "." in blueCardJuleicaEntitlement and goldenCardHonoredByMinisterPresidentEntitlement, but we have the "." for other items.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well i think the requirements are entire sentences so they should have a dot when the user selects a requirement.
I think its not really needed to strip that. But you are right it should be equal for all requirements

@seluianova seluianova removed their assignment Jan 13, 2025
@f1sh1918 f1sh1918 enabled auto-merge January 14, 2025 09:42
@f1sh1918 f1sh1918 merged commit f88da00 into main Jan 14, 2025
2 checks passed
@f1sh1918 f1sh1918 deleted the 1843-missing-application-labels branch January 14, 2025 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@seluianova seluianova seluianova approved these changes

@steffenkleinle steffenkleinle Awaiting requested review from steffenkleinle steffenkleinle is a code owner

@ztefanie ztefanie Awaiting requested review from ztefanie ztefanie is a code owner

@michael-markl michael-markl Awaiting requested review from michael-markl

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Typo in the online application form- Bavaria Label/Translation missing in Bavarian Volunteer Card
3 participants
@f1sh1918 @seluianova @michael-markl