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

Satisfaction Bot links #753

Closed
iammattmartin opened this Issue Sep 7, 2018 · 17 comments

Comments

2 participants
@iammattmartin

iammattmartin commented Sep 7, 2018

In v9, the links in the Satisfaction bot no longer link anywhere.

They link to javascript;; but before (v8) they took you to the ticket/message but now nothing.

@jstanden jstanden added this to the 9.0.4 milestone Sep 7, 2018

@jstanden

This comment has been minimized.

Owner

jstanden commented Sep 7, 2018

@iammattmartin Is this on one of the dashboards in the 'Satisfaction' page? We'll give it another test. The package was imported in 8.x and then you upgraded to 9.0, right?

@iammattmartin

This comment has been minimized.

iammattmartin commented Sep 7, 2018

That's correct.

On one of the feedbacks, where it links to the ticket in question. It is a link but to javascript;;.

Can confirm it was imported to v8, then upgraded to v9.

@iammattmartin

This comment has been minimized.

iammattmartin commented Sep 7, 2018

May be relevant. The "see more..." link also seems to go nowhere, same javascript;; ref.

@jstanden

This comment has been minimized.

Owner

jstanden commented Sep 7, 2018

The javascript:; hrefs are valid. There are some data-* attributes on those links that Cerb uses to "progressively enhance" the UI elements. The "see more..." link should open a search popup, and the other links on feedback should be opening cards.

Rather than direct links, they're .click() event handlers.

We're investigating this right now, along with checking the 8.x->9.0 upgrade path for any potential issues on that package.

In earlier versions, the 'Custom HTML' dashboard widgets were allowed to display any content/scripts, even when owned by a worker. This is a security issue, so those are locked down to just Cerb's permitted markup (using classes and data attribs on the elements). Whatever is going on here is going to be related to that.

@jstanden jstanden added this to In Development in 9.0.10 Sep 7, 2018

@jstanden

This comment has been minimized.

Owner

jstanden commented Sep 7, 2018

We can reproduce this. Getting a fix together.

@jstanden jstanden added the bug label Sep 7, 2018

@jstanden jstanden self-assigned this Sep 7, 2018

@jstanden

This comment has been minimized.

Owner

jstanden commented Sep 7, 2018

@iammattmartin Alright! Pull 9.0.4 from cerb/cerb-release.git and you should be all set.

Thanks for the report! (and apologies for the delay)

@jstanden jstanden closed this Sep 7, 2018

9.0.10 automation moved this from In Development to Completed! Sep 7, 2018

@iammattmartin

This comment has been minimized.

iammattmartin commented Sep 7, 2018

Hmm.. That did not go well.

Pressed the upgrade link as normal, got a "MySQL Error" and now see "Another administrator is currently running an update. Please wait..." in text but no one else is running it.

(MySQL has no errors and didn't die etc).

2018/09/07 22:41:50 [error] 12291#12291: *1616419 FastCGI sent in stderr: "PHP message: PHP Warning:  chmod(): No such file or directory in /var/www/libs/devblocks/api/services/bootstrap/cache.php on line 374" while reading response header from upstream, client: x.x.x.x, server: help.desk, request: "POST /rest/tickets/search.json HTTP/1.1", upstream: "fastcgi://unix:/tmp/php-fpm.sock:", host: "x.x"
2018/09/07 22:41:51 [error] 12298#12298: *1616301 FastCGI sent in stderr: "PHP message: [0]  ::SQL::
	CREATE TABLE `worker_dashboard_pref` (
		tab_context varchar(255) NOT NULL,
		tab_context_id int(10) unsigned NOT NULL,
		worker_id int(10) unsigned NOT NULL DEFAULT '0',
		widget_id int(10) unsigned NOT NULL DEFAULT '0',
		pref_key varchar(255) NOT NULL,
		pref_value varchar(255) DEFAULT NULL,
		PRIMARY KEY (`tab_context`,`tab_context_id`,`worker_id`,`widget_id`,`pref_key`)
	) ENGINE=MyISAM;
	" while reading response header from upstream, client: x.x.x, server: x.x.x, request: "GET /update HTTP/2.0", upstream: "fastcgi://unix:/tmp/php-fpm.sock:", host: "x.x.x", referrer: "https://x.x.x/update/locked"
2018/09/07 22:42:23 [error] 12297#12297: *1616453 FastCGI sent in stderr: "PHP message: PHP Warning:  chmod(): No such file or directory in /var/www/libs/devblocks/api/services/bootstrap/cache.php on line 374" while reading response
@iammattmartin

This comment has been minimized.

iammattmartin commented Sep 7, 2018

Trying that MySQL manually, @jstanden.

Specified key was too long; max key length is 1000 bytes
@jstanden

This comment has been minimized.

Owner

jstanden commented Sep 7, 2018

@iammattmartin MyISAM seems to be the problem there. Just when you think something is dead...

InnoDB has a max key length of 3500 bytes (effectively 3072 bytes).

Temporarily, you can remove the PRIMARY KEY in the patch for that table to appease MyISAM. It's not used directly for anything. Or you can halve the tab_context and pref_key lengths from 255 to 128.

Do you happen to have every table in utf8mb4?

@jstanden

This comment has been minimized.

@iammattmartin

This comment has been minimized.

iammattmartin commented Sep 7, 2018

They're all utf8. Have set to 128, removed the lock file and re-attempted which is fine.

I am guessing that $db->ExecuteMaster didn't run so should probably do that?

Now I am back in the links don't work yet so could be related.

@jstanden

This comment has been minimized.

Owner

jstanden commented Sep 7, 2018

@iammattmartin Yeah, if the patch died it wouldn't have run to that point.

You could do an UPDATE cerb_patch_history SET revision=revision-1 WHERE plugin_id = 'cerberusweb.core' in MySQL.

Then run /update again to finish the patch.

@iammattmartin

This comment has been minimized.

iammattmartin commented Sep 7, 2018

That worked a treat.

Our install is stock and I was contemplating adding innodb_large_prefix in to MySQL as that is meant to help increase the limit to over 3000 but as it isn't in the original cerb guide, I suspect it's not really a solution.

@jstanden

This comment has been minimized.

Owner

jstanden commented Sep 7, 2018

If you're using InnoDB for other things, make sure APP_DB_ENGINE is InnoDB in framework.config.php. That way Cerb will create new tables with InnoDB rather than MyISAM (your MySQL's default storage engine).

I'll push another patch to address the MyISAM issue. It looks like we'll still need to add that to the tests for a while longer.

Thanks!

@iammattmartin

This comment has been minimized.

iammattmartin commented Sep 7, 2018

Noted. Is it worth converting any MyISAM tables to InnoDB? I assume cerb will be fine with that?

@jstanden

This comment has been minimized.

Owner

jstanden commented Sep 7, 2018

Yeah, I'd recommend that to gain the InnoDB benefits. You won't need to do anything special on the Cerb side switching between those for existing tables.

@iammattmartin

This comment has been minimized.

iammattmartin commented Sep 7, 2018

Thanks. All done. You can ignore the email!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment