Skip to content

Make Sabre File exception messages translatable - #30536

Merged
Pytal merged 1 commit into
masterfrom
fix/l10n-exception-message
Mar 2, 2022
Merged

Make Sabre File exception messages translatable#30536
Pytal merged 1 commit into
masterfrom
fix/l10n-exception-message

Conversation

@Pytal

@PytalPytal commented Jan 8, 2022•
edited
Loading

Copy link
Copy Markdown
Member

See title 👆

@PytalPytal added bug 2. developing Work in progress feature: language/translations (l10n/i18n) Localization and translation matters labels Jan 8, 2022
CarlSchwan
CarlSchwan reviewed Jan 9, 2022
View reviewed changes
Comment threadapps/dav/lib/Connector/Sabre/File.php Outdated
Comment threadapps/dav/lib/Connector/Sabre/File.php Outdated
@Pytal
Pytalforce-pushed the fix/l10n-exception-message branch from a840650 to 0878363CompareJanuary 11, 2022 00:11
@PytalPytal changed the title Make some exception messages translatableMake Sabre File exception messages translatableJan 11, 2022
@PytalPytal added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jan 11, 2022
@PytalPytal added this to the Nextcloud 24 milestone Jan 11, 2022
@Pytal

Pytal commented Jan 11, 2022

Copy link
Copy Markdown
MemberAuthor

/backport to stable23

@Pytal

Pytal commented Jan 11, 2022

Copy link
Copy Markdown
MemberAuthor

/backport to stable22

@Pytal

Pytal commented Jan 11, 2022

Copy link
Copy Markdown
MemberAuthor

/backport to stable21

@Pytal
Pytal marked this pull request as ready for review January 11, 2022 00:13
@Pytal
Pytal requested review from a team, ArtificialOwl, CarlSchwan and nickvergessen and removed request for a teamJanuary 11, 2022 00:23
@PytalPytal self-assigned this Jan 11, 2022
CarlSchwan
CarlSchwan approved these changes Jan 14, 2022
View reviewed changes

@CarlSchwanCarlSchwan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could it be maybe possible to store $this->l10nFactory->get(Application::APP_ID) inside a property of the File class instead of calling this everytime?

@Pytal
Pytalforce-pushed the fix/l10n-exception-message branch from 0878363 to ee8f192CompareJanuary 14, 2022 19:23
nickvergessen
nickvergessen reviewed Jan 17, 2022
View reviewed changes
Comment threadapps/dav/lib/Connector/Sabre/File.php Outdated
@Pytal
Pytalforce-pushed the fix/l10n-exception-message branch 2 times, most recently from 18e8c6e to c53b587CompareFebruary 4, 2022 19:06
@PVince81

PVince81 commented Feb 17, 2022

Copy link
Copy Markdown
Member

need to check with clients to make sure they don't happen to read the english messages to distinguish errors

@tobiasKaminsky

@tobiasKaminsky

tobiasKaminsky commented Feb 18, 2022

Copy link
Copy Markdown
Member

need to check with clients to make sure they don't happen to read the english messages to distinguish errors

Which endpoint is this? Then I can check with team.

@Pytal

Pytal commented Feb 18, 2022

Copy link
Copy Markdown
MemberAuthor

Which endpoint is this? Then I can check with team.

This class is only used within the dav app so https://github.com/nextcloud/server/blob/master/apps/dav/appinfo/routes.php probably contains all of them

@tobiasKaminsky

tobiasKaminsky commented Feb 21, 2022

Copy link
Copy Markdown
Member

This class is only used within the dav app so https://github.com/nextcloud/server/blob/master/apps/dav/appinfo/routes.php probably contains all of them

If this is true, then there is no problem as we do only use v1/direct, which should be handled without translations.

@Pytal

Pytal commented Feb 22, 2022

Copy link
Copy Markdown
MemberAuthor

If this is true, then there is no problem as we do only use v1/direct, which should be handled without translations.

IDE reports use only within dav app, can you confirm @PVince81?

@PVince81

PVince81 commented Feb 23, 2022

Copy link
Copy Markdown
Member

If this is true, then there is no problem as we do only use v1/direct, which should be handled without translations.

IDE reports use only within dav app, can you confirm @PVince81?

I'm not familiar with this route. You could try grepping through the release tarball if you want to be sure.

@Pytal

Pytal commented Feb 23, 2022

Copy link
Copy Markdown
MemberAuthor

I'm not familiar with this route. You could try grepping through the release tarball if you want to be sure.

grepped through release tarball and looks good, @tobiasKaminsky can you confirm that the exception strings aren't used on clients?

@tobiasKaminsky

tobiasKaminsky commented Feb 24, 2022

Copy link
Copy Markdown
Member

Strings are not used, but only shown, e.g. if direct endpoint fails. Then they might be either shown as is or a generic error message is shown.
In either case we do not rely on exact wording of those strings, but only display them.

@Pytal
Pytalforce-pushed the fix/l10n-exception-message branch from c53b587 to 3355350CompareFebruary 24, 2022 17:51
@Pytal

Pytal commented Feb 24, 2022

Copy link
Copy Markdown
MemberAuthor

Rebased

@Pytal
Make Sabre File exception messages translatable
67ec981
Signed-off-by: Christopher Ng <chrng8@gmail.com>
@Pytal
Pytalforce-pushed the fix/l10n-exception-message branch from 3355350 to 67ec981CompareMarch 1, 2022 23:29
@PytalPytal added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Mar 1, 2022
@Pytal
Pytal merged commit 9d088df into masterMar 2, 2022
@Pytal
Pytal deleted the fix/l10n-exception-message branch March 2, 2022 00:11
This was referenced Mar 2, 2022
Merged
Merged
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@artongeartongeartonge approved these changes
@CarlSchwanCarlSchwanCarlSchwan approved these changes
@ArtificialOwlArtificialOwlAwaiting requested review from ArtificialOwl
@nickvergessennickvergessenAwaiting requested review from nickvergessen

Assignees

@PytalPytal

Labels

4. to releaseReady to be released and/or waiting for tests to finishbugfeature: language/translations (l10n/i18n)Localization and translation matters

Projects

None yet

Milestone

Nextcloud 24

Development

Successfully merging this pull request may close these issues.

6 participants

@Pytal@PVince81@tobiasKaminsky@nickvergessen@artonge@CarlSchwan