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

fixes #3430 ACP settings search option hardcoded only for english language #3438

Conversation

avril-gh
Copy link
Contributor

@avril-gh avril-gh commented Sep 3, 2018

Fixes issue where ACP settings search option, has searched directly DB data (which is in english) without considering translation. Because of this, ACP options search feature didnt worked for any other language than english.

… language.

Fixes issue where ACP settings search option, has searched directly DB
data (which is in english) without considering translation. Because of
this, ACP options search feature didnt worked for any other language
than english.
@avril-gh
Copy link
Contributor Author

avril-gh commented Sep 3, 2018

fixes #3430

@WildcardSearch
Copy link
Contributor

@avril-gh I have a little time to test this week.

Make sure I have steps to reproduce correct:

  • Load ACP using a translation
  • ACP->Config->Settings [search box]
  • current behavior (doesn't find setting because it is looking for English name)
  • expected behavior (searches for translated language when appropriate)

@WildcardSearch WildcardSearch added the s:confirmed Status: Confirmed. Retested and found the issue exists label Sep 18, 2018
@WildcardSearch
Copy link
Contributor

Nevermind, @avril-gh

Confirmed on MyBB 1.8.19

Copy link
Contributor

@WildcardSearch WildcardSearch left a comment

Choose a reason for hiding this comment

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

Tested in 1.8.19

Fixes the named issue and provides no side effects that I can see.

@avril-gh
Copy link
Contributor Author

If admin use in ACP another language than english, he obviously will name options in his own language, but since search box directly query on DB tables which are in english. he will not find the option he is looking for.

The solution is to not search directly on DB tables (like now) but instead, fetch them and search them localy while applying translation in the fly.

like usual, tested closely at each stage before submitting PR
Thanks for attention @WildcardSearch

@WildcardSearch
Copy link
Contributor

Thank you, @avril-gh.

I was able to reproduce the error. Your PR solves the issue. Maybe @dvz or @euantorano will have time to merge it.

Copy link
Contributor Author

@avril-gh avril-gh left a comment

Choose a reason for hiding this comment

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

because of data in DB is by default hardcoded in english
1148 - we'll fetch settings
1151 - instead of searching it directly on DB
1155 - and dynamicaly translate -> search -> build and returns $cache_settings variable in exacly same format as it's expected by later logic flow, so theres no need for more changes.

@WildcardSearch WildcardSearch merged commit 9065208 into mybb:feature Oct 10, 2018
@WildcardSearch WildcardSearch added this to the 1.8.20 milestone Oct 10, 2018
@WildcardSearch
Copy link
Contributor

Fixes #3430

@avril-gh avril-gh deleted the fixes-#3430-ACP-settings-search-option-hardcoded-only-for-english-language- branch October 11, 2018 09:34
@effone effone removed this from the 1.8.20 milestone Jan 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
b:1.8 Branch: 1.8.x s:confirmed Status: Confirmed. Retested and found the issue exists
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants