Skip to content

Techdebt/noid/dedicated method to get a users language - #21533

Merged
rullzer merged 2 commits into
masterfrom
techdebt/noid/dedicated-method-to-get-a-users-language
Jul 4, 2020
Merged

Techdebt/noid/dedicated method to get a users language#21533
rullzer merged 2 commits into
masterfrom
techdebt/noid/dedicated-method-to-get-a-users-language

Conversation

@nickvergessen

@nickvergessennickvergessen commented Jun 22, 2020

Copy link
Copy Markdown
Member

@nickvergessennickvergessen added 2. developing Work in progress feature: language/translations (l10n/i18n) Localization and translation matters labels Jun 22, 2020
@nickvergessennickvergessen added this to the Nextcloud 20 milestone Jun 22, 2020

@rullzerrullzer 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.

code looks good.
didn't test. But I can see the bug this solves.

@blizzz

Copy link
Copy Markdown
Member

Mh, what about making the LanguageIterator accepting null-users and just returning it's first value?

@nickvergessen

Copy link
Copy Markdown
MemberAuthor

I guess it also should take an appid then as that could influence the result too.
But yeah this was just a end of workday push, i will continue tomorrow.

Also i never saw this iterator before and parta of the fix need backporting till 17 :/

@blizzz

Copy link
Copy Markdown
Member

I guess it also should take an appid then as that could influence the result too.
But yeah this was just a end of workday push, i will continue tomorrow.

Yeah, could be integrated right away, too.

Also i never saw this iterator before and parta of the fix need backporting till 17 :/

I added it for the what's new thing. Haven't seen that anyone picked it up, but it was also rather a corner case.

@nickvergessen
nickvergessenforce-pushed the techdebt/noid/dedicated-method-to-get-a-users-language branch from 8e0d34d to 7e28104CompareJune 23, 2020 06:48
@nickvergessennickvergessen added the pending documentation This pull request needs an associated documentation update label Jun 23, 2020
@nickvergessen

Copy link
Copy Markdown
MemberAuthor

Now based on #21535 which is backportable easily
And using the new method everywhere.

I'm not sure about the iterator usage, especially as it falls back to the current user and therefor e.g. always sends share related emails in the sharers language instead of default for guests.

@nickvergessen
nickvergessenforce-pushed the techdebt/noid/dedicated-method-to-get-a-users-language branch from 7e28104 to aafcffcCompareJune 23, 2020 06:53
@nickvergessen
nickvergessenforce-pushed the techdebt/noid/dedicated-method-to-get-a-users-language branch from aafcffc to 6445bc9CompareJune 24, 2020 11:34
@nickvergessennickvergessen added 4. to release Ready to be released and/or waiting for tests to finish and removed 2. developing Work in progress labels Jun 24, 2020
@rullzer

Copy link
Copy Markdown
Member

Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen
nickvergessenforce-pushed the techdebt/noid/dedicated-method-to-get-a-users-language branch from 6445bc9 to 44f33eaCompareJuly 1, 2020 08:35
@rullzer

Copy link
Copy Markdown
Member

CI still says no

Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen
nickvergessenforce-pushed the techdebt/noid/dedicated-method-to-get-a-users-language branch from 44f33ea to db8267dCompareJuly 3, 2020 08:59
@nickvergessen

Copy link
Copy Markdown
MemberAuthor

Fixed

@rullzer
rullzer merged commit 4d8b4ef into masterJul 4, 2020
@rullzer
rullzer deleted the techdebt/noid/dedicated-method-to-get-a-users-language branch July 4, 2020 06:56
@nickvergessen

Copy link
Copy Markdown
MemberAuthor

Docs at nextcloud/documentation#9309

@nickvergessennickvergessen removed the pending documentation This pull request needs an associated documentation update label Nov 9, 2022
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@nickvergessen@blizzz@rullzer