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

When viewing graphs, shifting time does not work when using non-english languages #3520

Closed
Susanin63 opened this issue Apr 29, 2020 · 13 comments
Labels
bug Undesired behaviour confirmed Bug is confirm by dev team resolved A fixed issue
Milestone

Comments

@Susanin63
Copy link
Contributor

Describe the bug

If you use non english language time shift on graph tab don't work. At any shift time goes to 1970.

To Reproduce

Steps to reproduce the behavior:
Change language in user's profile to any not English

  1. Go to 'Graph' tab

  2. Click on 'time shift left'

  3. See error in datetime field

The problem is array $graph_timeshifts
function strtotime use ONLY english timestamps:
strtotime("-2 Day 2020-04-28 16:26")
: long = 1587903960

strtotime("-2 День 2020-04-28 16:26")
: bool = FALSE

@Susanin63 Susanin63 added bug Undesired behaviour unverified Some days we don't have a clue labels Apr 29, 2020
@netniV
Copy link
Member

netniV commented Apr 29, 2020

So basically, we need to use the i18n version of this to convert properly?

@Susanin63
Copy link
Contributor Author

But there is none: https://stackoverflow.com/questions/6988536/strtotime-with-different-languages
May be create second $graph_timeshifts on clear english and use it on set_timeshift() function ?

@netniV
Copy link
Member

netniV commented Apr 29, 2020

Does the DateTime class have the same limitation? If not, that should be used instead. I'm sure that's what I have used with one of my RackTable plugins.

@netniV
Copy link
Member

netniV commented Apr 29, 2020

Actually, thinking more on this, if DateTime works you should use DateTime->getTimestamp() to verify.

php -a
php > $d = new DateTime();
php > $d2 = $d->modify('-2 days');
php > echo $d2->getTimestamp();
1587999647

@Susanin63
Copy link
Contributor Author

$d = new DateTime();
: object(DateTime) =
date: string = "2020-04-29 20:20:08.477594"
timezone_type: long = 3
timezone: string = "Europe/Samara"
$d2 = $d->modify('-2 дня');
: bool = FALSE

@Susanin63
Copy link
Contributor Author

All functions work only with english words

@netniV
Copy link
Member

netniV commented Apr 29, 2020

Damn, there was a hope... So we need to reverse engineer the i18n language components... not sure how we can do that in the current code without a lot of hard coding, @TheWitness any ideas?

@netniV

This comment has been minimized.

@netniV

This comment has been minimized.

@netniV
Copy link
Member

netniV commented Apr 29, 2020

Right so I can reproduce this by setting my user language to Espanol... what I can't do so far is find out why. Everything appears to be passing around integers from the GUI end, so something in the code must be translating that using the $graph_timespan setting but I haven't found the exact line yet...

@netniV netniV changed the title On non-english language time shift on graphs don't work When viewing graphs, shifting time does not work when using non-english languages Apr 29, 2020
netniV added a commit that referenced this issue Apr 29, 2020
@netniV
Copy link
Member

netniV commented Apr 29, 2020

Finally found the problem in lib/timespan_settings.php which is not a file I have ventured into before. When it first initailizes it calls set_timeshift() before passing that result onto process_user_input().

/* set variables for first time use */
initialize_timespan($timespan);
$timeshift = set_timeshift();

/* if the user does not want to see timespan selectors */
process_html_variables();
process_user_input($timespan, $timeshift);

This seems innocuous until you realise that it is using the integer value of sess_current_timeshift (which currently has the value not the text). But then uses that to retrieve the text version from graph_timeshifts which is the global variable holding the i18n version of the text.

/* establish graph timeshift from either a user select or the default */
function set_timeshift() {
        global $config, $graph_timeshifts;

        # no current timeshift: get default timeshift
        if ((!isset($_SESSION['sess_current_timeshift'])) ||
                (isset_request_var('button_clear')) 
                ) {
                $_SESSION['sess_current_timeshift'] = read_user_setting('default_timeshift');
                set_request_var('predefined_timeshift', read_user_setting('default_timeshift'));
                $_SESSION['custom'] = 0;
        }

        return $timeshift = $graph_timeshifts[$_SESSION['sess_current_timeshift']];
}

It is very likely that at some point, one of us looked at the variable and said "Hey, why isn't that in the correct language on the display" and simply wrapped it in the i18n call rather than an sprintf, which unfortunately, results in the call looking like:

2020/04/29 22:17:29 - CMDPHP DEBUG: shift_time(span: {"current_value_date1":"2020-04-28 22:17","current_value_date2":"2020-04-29 22:17","begin_now":1588108638,"end_now":1588195038}, direction: +, shift_size: 1 dia)

So, I have now created a second var called graph_timeshift_vals that holds the textual values required by PHP to be in English:

2020/04/29 22:25:09 - CMDPHP DEBUG: shift_time(span: {"current_value_date1":"2020-04-28 22:17","current_value_date2":"2020-04-29 22:17","begin_now":1588108620,"end_now":1588195020}, direction: +, shift_size: 1 Day)

@netniV netniV added confirmed Bug is confirm by dev team and removed unverified Some days we don't have a clue labels Apr 29, 2020
@netniV netniV added this to the 1.2.12 milestone Apr 29, 2020
@Susanin63
Copy link
Contributor Author

Thats fix it!

@netniV
Copy link
Member

netniV commented Apr 30, 2020

Hopefully @TheWitness or @cigamit can approve and merge my change for the release then

@netniV netniV added the resolved A fixed issue label May 3, 2020
@netniV netniV closed this as completed May 3, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Aug 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Undesired behaviour confirmed Bug is confirm by dev team resolved A fixed issue
Projects
None yet
Development

No branches or pull requests

2 participants