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

Add reviewer modal expanding reviewer details javascript error #6024

Closed
procky opened this issue Jun 19, 2020 · 6 comments
Closed

Add reviewer modal expanding reviewer details javascript error #6024

procky opened this issue Jun 19, 2020 · 6 comments
Labels
Bug:1:Low A bug that does not have a severe consequence or affects a small number of users.

Comments

@procky
Copy link

procky commented Jun 19, 2020

In the modal when you click the expand arrow for more details on a reviewer nothing happens and an error appears in the console.

Javascript Error
TypeError: "e is null"
in build.js

I have found in the data given to the smarty template if the biography for a reviewer is null it will fail but if it is an empty string or array of locales it works.

Error reproducible on our journal (OJS 3.2.0.1) and also tested using a fresh docker-ojs install.

Reproduce the behaviour:

  1. Install OJS. Only noteworthy thing is that I set it up to use 2 locales.
  2. Setup an author by registering like a real external author would (/user/register)
  3. Logged in as admin I made an editor user too
  4. Submit a new submission as the author
  5. As admin assign the editor
  6. Move the submission to review
  7. Open the add reviewer modal and the reviewer will not expand
@NateWr
Copy link
Contributor

NateWr commented Jun 30, 2020

@procky I'm looking into this now but I'm unable to reproduce the issue. It looks like this was addressed as part of the 3.1.2-4 release: pkp/ui-library@1d17444

Specifically, it is this line https://github.com/pkp/ui-library/blob/master/src/components/ListPanel/users/SelectReviewerListItem.vue#L175-L180 where the biography is loaded and that localize() call should work correctly even if biography is null.

I'd recommend trying with a newer release to see if this resolves your issue.

@NateWr NateWr closed this as completed Jun 30, 2020
@awecancer
Copy link

awecancer commented Jul 2, 2020

@NateWr thank you for looking at it. Helped point me in the right direction.

Disclosure: I am Procky but I am now using this GitHub account.

I can reproduce this error with fresh installs of OJS and can now see the problem. We have users created from registration and they do not get given a biography in the user_settings table. We upgrade our OJS journal using the tar.gz files from https://pkp.sfu.ca/ojs/ojs_download/ and in the latest version (3.2.1-1) the localize function is:

localize:function(e,t){if("undefined"===typeof e)return"";if(void 0!==t)return e.hasOwnProperty(t)?e[t]:"";if(e.hasOwnProperty($.pkp.app.currentLocale)&&e[$.pkp.app.currentLocale])return e[$.pkp.app.currentLocale];if(e.hasOwnProperty($.pkp.app.primaryLocale)&&e[$.pkp.app.primaryLocale])return e[$.pkp.app.primaryLocale];for(var n in e)if(e[n])return e[n];return""}

From what I can tell this matches https://github.com/pkp/ui-library/blob/master/src/mixins/global.js#L104
and unfortunately not the localize function you linked.

From debugging build.js I can see the code does as expected. First localized call is for affiliation and the first param is an object with the locale affiliation string. The second call the param is null and is called for biography. It fails when checking the third if statement.

Your localize function's null check looks good to solve this issue.

@NateWr
Copy link
Contributor

NateWr commented Jul 2, 2020

👍 Good detective work, @awecancer aka Procky. It looks like this has changed between stable-3_2_0 and master, due to #5865.

This is the function as it appears in 3.2.0 and 3.2.1, with the typeof check rather than the !multilingual, which babel will compile down to include a null check: https://github.com/pkp/ui-library/blob/stable-3_2_0/src/mixins/global.js#L70.

I'll file this to be fixed for our next 3.2.1.x release.

If you're willing, we'd love to have a PR against the stable-3_2_1 branch of UI Library that addresses this. I think changing the first if conditional to match what's in master would do the trick.

If you wanted to fix it in your local install by editing the build.js file directly, you can probably do it by changing:

if("undefined"===typeof e)

To:

if("undefined"===typeof e || "null"===typeof e)

@NateWr NateWr reopened this Jul 2, 2020
@NateWr NateWr added the Bug:1:Low A bug that does not have a severe consequence or affects a small number of users. label Jul 2, 2020
@NateWr NateWr added this to the OJS/OMP/OPS 3.2.1-2 milestone Jul 2, 2020
@NateWr
Copy link
Contributor

NateWr commented Jul 13, 2020

PR:
pkp/ui-library#109 (stable-3_2_1)

Tests:
pkp/ojs#2836 (stable-3-2_1)

@awecancer
Copy link

Thanks @jnugent. Also while improbable anyone will need this, the direct change to the build.js file would be:

if("undefined"===typeof e || null===e)

NateWr added a commit to pkp/ojs that referenced this issue Jul 14, 2020
Submodule update for pkp/pkp-lib#6024 localization fix in reviewer list panel
@NateWr
Copy link
Contributor

NateWr commented Jul 14, 2020

Thanks everyone, that's been merged to the stable-3_2_1 branch now.

@NateWr NateWr closed this as completed Jul 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug:1:Low A bug that does not have a severe consequence or affects a small number of users.
Projects
None yet
Development

No branches or pull requests

3 participants