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

Localize issue and article cover images #1934

Closed
bozana opened this issue Nov 1, 2016 · 19 comments
Closed

Localize issue and article cover images #1934

bozana opened this issue Nov 1, 2016 · 19 comments
Assignees
Milestone

Comments

@bozana
Copy link
Collaborator

bozana commented Nov 1, 2016

The issue cover images are not localized any more in OJS 3.0, which is a problem for journals migrating from 2.4.x, s. for example http://forum.pkp.sfu.ca/t/update-to-ojs-3-failed/21564. Thus the issue cover images should be saved localized again.
The same guilt for article cover images.
The monograph cover images are not localized. If necessary, this can be changed later, as a separate issue.

@bozana
Copy link
Collaborator Author

bozana commented Nov 2, 2016

PRs:
pkp-lib master: #1952
ojs master: pkp/ojs#1083
omp master: pkp/omp#346

pkp-lib ojs-stable-3_0_0: #1972
ojs ojs-stable-3_0_0: pkp/ojs#1092

@bozana
Copy link
Collaborator Author

bozana commented Nov 2, 2016

@NateWr, could you take a look at this PR -- because you have already worked on that? This is just issue cover image localization, I will do the article cover image localization tomorrow...

@bozana
Copy link
Collaborator Author

bozana commented Nov 3, 2016

Ah, sorry @NateWr, could you please wait with this -- I forgot to consider import/export :-(

@bozana bozana changed the title Localize issue cover images Localize issue and article cover images Nov 3, 2016
bozana added a commit to bozana/pkp-lib that referenced this issue Nov 3, 2016
bozana added a commit to bozana/ojs that referenced this issue Nov 3, 2016
bozana added a commit to bozana/ojs that referenced this issue Nov 3, 2016
bozana added a commit to bozana/ojs that referenced this issue Nov 3, 2016
bozana added a commit to bozana/ojs that referenced this issue Nov 3, 2016
bozana added a commit to bozana/omp that referenced this issue Nov 3, 2016
bozana added a commit to bozana/omp that referenced this issue Nov 3, 2016
bozana added a commit to bozana/pkp-lib that referenced this issue Nov 3, 2016
bozana added a commit to bozana/ojs that referenced this issue Nov 3, 2016
bozana added a commit to bozana/omp that referenced this issue Nov 3, 2016
bozana added a commit to bozana/omp that referenced this issue Nov 3, 2016
bozana added a commit to bozana/ojs that referenced this issue Nov 3, 2016
bozana added a commit to bozana/ojs that referenced this issue Nov 3, 2016
bozana added a commit to bozana/ojs that referenced this issue Nov 3, 2016
bozana added a commit to bozana/ojs that referenced this issue Nov 3, 2016
@bozana
Copy link
Collaborator Author

bozana commented Nov 3, 2016

@NateWr, the PRs above are ready for code review :-) Thanks a lot!

@NateWr
Copy link
Contributor

NateWr commented Nov 7, 2016

Looks good @bozana! I had a couple of very minor code style questions, but feel free to merge if you disagree.

I do think that we'll probably get some UX/UI feedback at some point from people who don't want to have to switch languages to upload alternate cover images. But I'm happy with this as a good-enough UX for now.

@bozana
Copy link
Collaborator Author

bozana commented Nov 7, 2016

Thanks a lot @NateWr, I'll make the suggested changes and let you know. Then I will cherry-pick it to ojs-stable-3_0_0 as well...

@NateWr
Copy link
Contributor

NateWr commented Nov 28, 2016

@bozana I'm having some trouble with localized article images. I'm trying to update the bootstrap theme to support hte localized images: https://github.com/NateWr/bootstrap3/issues/43

However, I'm getting a Fatal Error when $article->getLocalizedCoverImage() is called on an article with an image that was uploaded before this change was in place.

[Mon Nov 28 18:59:19.233344 2016] [:error] [pid 22812] [client 192.168.0.8:59348] PHP Warning:  Declaration of SubmissionFileDAO::fromRow($row) should be compatible with PKPSubmissionFileDAO::fromRow($row, $fileImplementation) in /var/www/html/ojs/classes/article/SubmissionFileDAO.inc.php on line 23, referer: http://192.168.0.8/ojs/index.php/publicknowledge/article/view/10
[Mon Nov 28 18:59:19.235788 2016] [:error] [pid 22812] [client 192.168.0.8:59348] PHP Warning:  Cannot use a scalar value as an array in /var/www/html/ojs/lib/pkp/classes/core/DataObject.inc.php on line 133, referer: http://192.168.0.8/ojs/index.php/publicknowledge/article/view/10
[Mon Nov 28 18:59:19.235799 2016] [:error] [pid 22812] [client 192.168.0.8:59348] PHP Warning:  Cannot use a scalar value as an array in /var/www/html/ojs/lib/pkp/classes/core/DataObject.inc.php on line 133, referer: http://192.168.0.8/ojs/index.php/publicknowledge/article/view/10
[Mon Nov 28 18:59:19.235805 2016] [:error] [pid 22812] [client 192.168.0.8:59348] PHP Warning:  Cannot use a scalar value as an array in /var/www/html/ojs/lib/pkp/classes/core/DataObject.inc.php on line 133, referer: http://192.168.0.8/ojs/index.php/publicknowledge/article/view/10
[Mon Nov 28 18:59:19.235813 2016] [:error] [pid 22812] [client 192.168.0.8:59348] PHP Warning:  Cannot use a scalar value as an array in /var/www/html/ojs/lib/pkp/classes/core/DataObject.inc.php on line 133, referer: http://192.168.0.8/ojs/index.php/publicknowledge/article/view/10
[Mon Nov 28 18:59:19.235818 2016] [:error] [pid 22812] [client 192.168.0.8:59348] PHP Warning:  Cannot use a scalar value as an array in /var/www/html/ojs/lib/pkp/classes/core/DataObject.inc.php on line 133, referer: http://192.168.0.8/ojs/index.php/publicknowledge/article/view/10
[Mon Nov 28 18:59:19.235822 2016] [:error] [pid 22812] [client 192.168.0.8:59348] PHP Warning:  Cannot use a scalar value as an array in /var/www/html/ojs/lib/pkp/classes/core/DataObject.inc.php on line 133, referer: http://192.168.0.8/ojs/index.php/publicknowledge/article/view/10
[Mon Nov 28 18:59:19.260929 2016] [:error] [pid 22812] [client 192.168.0.8:59348] PHP Warning:  array_keys() expects parameter 1 to be array, string given in /var/www/html/ojs/lib/pkp/classes/submission/Submission.inc.php on line 153, referer: http://192.168.0.8/ojs/index.php/publicknowledge/article/view/10
[Mon Nov 28 18:59:19.260946 2016] [:error] [pid 22812] [client 192.168.0.8:59348] PHP Warning:  array_shift() expects parameter 1 to be array, null given in /var/www/html/ojs/lib/pkp/classes/submission/Submission.inc.php on line 154, referer: http://192.168.0.8/ojs/index.php/publicknowledge/article/view/10
[Mon Nov 28 18:59:19.260992 2016] [:error] [pid 22812] [client 192.168.0.8:59348] PHP Fatal error:  Uncaught Error: Cannot return string offsets by reference in /var/www/html/ojs/lib/pkp/classes/submission/Submission.inc.php:154\nStack trace:\n#0 /var/www/html/ojs/classes/article/Article.inc.php(253): Submission->getLocalizedData('coverImage')\n#1 /var/www/html/ojs/cache/t_compile/e26d2de610226743d9feff244fb14ef53700227f^%%A0^A0A^A0AA4E57%%article_summary.tpl.php(11): Article->getLocalizedCoverImage()\n#2 /var/www/html/ojs/lib/pkp/lib/vendor/smarty/smarty/libs/Smarty.class.php(1870): include('/var/www/html/o...')\n#3 /var/www/html/ojs/lib/pkp/classes/template/PKPTemplateManager.inc.php(359): Smarty->_smarty_include(Array)\n#4 /var/www/html/ojs/cache/t_compile/e26d2de610226743d9feff244fb14ef53700227f^%%DD^DDF^DDF03D78%%issue_toc.tpl.php(122): PKPTemplateManager->_smarty_include(Array)\n#5 /var/www/html/ojs/lib/pkp/lib/vendor/smarty/smarty/libs/Smarty.class.php(1870): include('/var/www/html/o...')\n#6 /var/www/html/ojs/lib/pkp/classes/template/PKPTemplateManager.inc.php(359): Smarty->_smarty_include(Array)\n#7 /var/ww in /var/www/html/ojs/lib/pkp/classes/submission/Submission.inc.php on line 154, referer: http://192.168.0.8/ojs/index.php/publicknowledge/article/view/10

I also found that I don't see the existing image in the article metadata. And if I try to upload a new image it doesn't work.

The cover images work fine on an article that's never had an image uploaded before. Do I need to run a database upgrade somewhere?

@NateWr
Copy link
Contributor

NateWr commented Nov 29, 2016

I was able to upgrade the database, though I had to manually set the core version to 3.0.0 and then upgrade to 3.0.1 (php tools/upgrade.php upgrade). However, the old cover image data doesn't seem to have been upgraded.

Here's a database entry for two submissions. ID 9 has a cover image uploaded before the changes and ID 10 has one uploaded after the changes:

+---------------+--------+-------------------+----------------------------------------------------+--------------+
| submission_id | locale | setting_name      | SUBSTRING(setting_value,1,50)                      | setting_type |
+---------------+--------+-------------------+----------------------------------------------------+--------------+
|             9 |        | coverImage        | article_9_cover.jpg                                | string       |
|             9 |        | coverImageAltText | Alt text here                                      | string       |
|            10 |        | coverImage        | a:2:{s:5:"en_US";s:26:"article_10_cover_en_US.jpg" | object       |
|            10 |        | coverImageAltText |                                                    | string       |
|            10 | en_US  | coverImage        | article_10_cover_en_US.jpg                         | string       |
|            10 | fr_CA  | coverImage        | article_10_cover_fr_CA.jpg                         | string       |
+---------------+--------+-------------------+----------------------------------------------------+--------------+
6 rows in set (0.00 sec)

Did I miss an upgrade routine somewhere?

@asmecher asmecher reopened this Nov 29, 2016
@asmecher
Copy link
Member

@bozana is probably not going to be available for a few days -- reopening the issue for a bit.

@bozana
Copy link
Collaborator Author

bozana commented Dec 1, 2016

@NateWr, this upgrade script change should actually consider the old and unlocalized cover images: bozana/ojs@493d18d#diff-694ad44b2764303882d41942f9516d5e and https://github.com/pkp/ojs/blob/master/dbscripts/xml/upgrade.xml#L114. Thus, I wonder why this do not change the submission 9 :-O And I wonder since when is the other entry with the array value for the coverImage for submission 10 there -- because also in the OJS 2.4.8 it is different i.e. the same as in 3.0 just that the setting name is different :-O Could you maybe also send me your DB dump? -- I could then take a look... THANKS!!!

bozana pushed a commit to bozana/ojs that referenced this issue Dec 7, 2016
@bozana
Copy link
Collaborator Author

bozana commented Dec 7, 2016

These issues should be considered too:
-- remove strange old cover images with array values in the DB - from 3.alpha or 3.beta?
-- remove empty 3.0 cover images
-- remove the old 2.4.x cover images, for which a new cover image exists

PR: pkp/ojs#1128

@bozana
Copy link
Collaborator Author

bozana commented Dec 7, 2016

@NateWr, could you code review and test the PR above? Just change the version to 3.0.0.0 before doing the DB update... THANKS!

@NateWr
Copy link
Contributor

NateWr commented Dec 7, 2016

@bozana I ran into an error when running the upgrade.

[data: dbscripts/xml/upgrade/3.0.1_update.xml]
ERROR: Upgrade failed: DB: Duplicate entry 'googlescholarplugin-6-enabled' for key 'plugin_settings_pkey'

I set the version to 3.0.0.0 in the database, then went to /tools and ran php upgrade.php upgrade. Is it a problem since I maybe already upgrade this database in teh past?

@bozana
Copy link
Collaborator Author

bozana commented Dec 7, 2016

Ah, yes, I also always have to comment out this line: https://github.com/pkp/ojs/blob/master/dbscripts/xml/upgrade.xml#L110
But that is not the right solution :-P @asmecher, is there a way to say: "If not exist, insert..." here: https://github.com/pkp/ojs/blob/master/dbscripts/xml/upgrade/3.0.1_update.xml#L15-L16, to make it idempotent? Something like MySQL INSERT IGNORE, REPLACE, or INSERT … ON DUPLICATE KEY UPDATE. Hmmm... I'll take a look...

@NateWr
Copy link
Contributor

NateWr commented Dec 7, 2016

Great! The upgrade worked this time and the cover image issue is fixed. 👍

@bozana
Copy link
Collaborator Author

bozana commented Dec 7, 2016

Great, thanks @NateWr! I will then merge it...
@asmecher, are those SQLs fine with PostgreSQL?

@asmecher, how about one of the following SQL statements, for solving the idempotent issue above:

INSERT INTO plugin_settings (plugin_name, setting_name, setting_value, setting_type, context_id) SELECT 'googlescholarplugin', 'enabled', '1', 'bool', j.journal_id FROM journals j
WHERE NOT EXISTS (SELECT ps.plugin_name FROM plugin_settings ps WHERE ps.plugin_name = 'googlescholarplugin' AND ps.context_id = j.journal_id)
INSERT INTO plugin_settings (plugin_name, setting_name, setting_value, setting_type, context_id) SELECT 'googlescholarplugin', 'enabled', '1', 'bool', j.journal_id FROM journals j
WHERE 'googlescholarplugin' NOT IN (SELECT ps.plugin_name FROM plugin_settings ps WHERE ps.plugin_name = 'googlescholarplugin' AND ps.context_id = j.journal_id)

?

@asmecher
Copy link
Member

asmecher commented Dec 7, 2016

@bozana, yes, both those queries run OK for me.

bozana added a commit to pkp/ojs that referenced this issue Dec 7, 2016
pkp/pkp-lib#1934 consider old cover images duplicates
bozana pushed a commit to bozana/ojs that referenced this issue Dec 7, 2016
@bozana
Copy link
Collaborator Author

bozana commented Dec 7, 2016

PR for the plugin settings insert statement:
master: pkp/ojs#1129
ojs-stable-3_0_1: pkp/ojs#1130

bozana pushed a commit to bozana/ojs that referenced this issue Dec 7, 2016
bozana pushed a commit to bozana/ojs that referenced this issue Dec 7, 2016
@bozana
Copy link
Collaborator Author

bozana commented Dec 8, 2016

Everything merged, thus closing...

@bozana bozana closed this as completed Dec 8, 2016
jprk pushed a commit to jprk/ojs that referenced this issue Dec 15, 2016
jprk pushed a commit to jprk/ojs that referenced this issue Dec 15, 2016
jprk pushed a commit to jprk/ojs that referenced this issue Dec 15, 2016
jprk pushed a commit to jprk/ojs that referenced this issue Dec 15, 2016
jprk pushed a commit to jprk/ojs that referenced this issue Dec 15, 2016
jprk pushed a commit to jprk/pkp-lib that referenced this issue Dec 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants