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

4679 javascript bundle #5351

Merged
merged 19 commits into from
Apr 5, 2019
Merged

Conversation

JayanthyChengan
Copy link
Contributor

@coveralls
Copy link

coveralls commented Nov 28, 2018

Coverage Status

Coverage remained the same at 17.769% when pulling 2195685 on scholarsportal:4679-javascript-bundle into ff9b01a on IQSS:develop.

Copy link
Contributor

@mheppler mheppler left a comment

Choose a reason for hiding this comment

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

@JayanthyChengan this all looks good. Going to move it back to Community Dev however and ask that you update this branch with all the other internationalization branches you have in motion currently. Specifically the #5202 alert msg branch which contains javascript fixes that should be tested here as part of #4679.

@JayanthyChengan
Copy link
Contributor Author

@mheppler noticed under My Data tab, there are still English terms not toggling with language selection
https://localhost:8080/dataverseuser.xhtml?selectTab=dataRelatedToMe

I am wondering where does these come from?
screen shot 2018-12-05 at 3 13 27 pm

@mheppler
Copy link
Contributor

mheppler commented Dec 5, 2018

@JayanthyChengan I've got some answers for you on where to locate those text examples you've highlighted above.

Account pg > My Data

  • "1 to 10 of 67 Results" comes from the template file found at /webapp/mydata_templates/result_message_only.html
  • Publication status labels like "Published" and "Draft" can be found at /webapp/mydata_templates/cards_minimum.html
  • Roles labels like "Admin" and "Curator" is coming from the getSelectedFilterParamsAsJSON() function which can be found in the MyDataFinder.java which pulls it from the database, which you can get from the bundle with the db name as the key like you've done previously in other beans
  • "DRAFT VERSION" in the citation is coming the Solr index which treats it the same as "data" which doesn't get translated

Let me know if you have any more My Data or javascript questions. It's been a long while since I've looked at this code, but I'd be glad to help sort through it all again to get you pointed in the right direction.

@JayanthyChengan
Copy link
Contributor Author

@mheppler
Item 1 and Item 2:
noticed that mydata_templates is using Nunjucks templating ( it's new to me ) , do I need to follow
https://github.com/KSDaemon/nunjucks-intl to pass bundle variables?

Item 3:
To get role names from bundle, tried the below snippet, but the language is not toggling in MyDataFinder.java when selected from language dropdown list.

`public JsonArrayBuilder getListofSelectedRoles(){

    JsonArrayBuilder jsonArray = Json.createArrayBuilder();
    
    for (Long roleId : this.filterParams.getRoleIds()){
        try {
            System.out.println("=========role." + this.rolePermissionHelper.getRoleName(roleId).toLowerCase().replace(" ", "") + ".name");
            String role = BundleUtil.getStringFromPropertyFile("role." + this.rolePermissionHelper.getRoleName(roleId).toLowerCase().replace(" ", "") + ".name", "BuiltInRoles");
            jsonArray.add(role);
        } catch (Exception e)
        {
            jsonArray.add(this.rolePermissionHelper.getRoleName(roleId));
        }

    }
    return jsonArray;                
}`

@mheppler
Copy link
Contributor

@JayanthyChengan

Yes, after reviewing the options with the dev team, the best way to get bundle text into the My Data html template files is by passing variables. I would add them to mydata_fragment.xhtml, where the init_mydata_page() function is called.

As for the role names, I have asked @scolapasta to provide some feedback for you.

@scolapasta
Copy link
Contributor

@JayanthyChengan
The first thing I'd try is adding the jsf:view tag to the user page like we had added for the alert messages. (basically it seems to me that the page is not getting the right locale)

@JayanthyChengan
Copy link
Contributor Author

JayanthyChengan commented Dec 12, 2018

@mheppler - I tried by passing hidden variables from mydata_fragment.xhtml , and used it in mydata.js to toggle using bundle value. Please check in this commit 507c82f

I noticed that from this link => http://localhost:8080/dataverseuser.xhtml?selectTab=dataRelatedToMe , all tabs are working fine.

But if I click on "Edit Account -> Account Information or Password" on the right side , I see the below error in server log

[2018-12-12T16:53:10.777-0500] [glassfish 4.1] [SEVERE] [] [javax.enterprise.resource.webcontainer.jsf.context] [tid: _ThreadID=32 _ThreadName=http-listener-1(4)] [timeMillis: 1544651590777] [levelValue: 1000] [[
javax.faces.component.UpdateModelException: javax.el.PropertyNotWritableException: /mydata_fragment.xhtml @35,79 value="#{bundle['mydata.result']}": ResourceBundles are immutable
at javax.faces.component.UIInput.updateModel(UIInput.java:866)
at javax.faces.component.UIInput.processUpdates(UIInput.java:749)
at javax.faces.component.UIComponentBase.processUpdates(UIComponentBase.java:1286)
at org.primefaces.component.api.UITabPanel.processUpdates(UITabPanel.java:1108)
at org.primefaces.component.tabview.TabView.processUpdates(TabView.java:333)

And it doesn't throw error , if I click "Edit Account -> Account Information or Password" from this link http://localhost:8080/dataverseuser.xhtml?selectTab=accountInfo

@JayanthyChengan
Copy link
Contributor Author

@scolapasta
I merged the "alert messages branch" which has jsf:view in all pages, and tested it, but still the language set to "en" and found out that mydata.js calls this below api to get data
http://localhost:8080/api/v1/mydata/retrieve?selected_page=1&dvobject_types=Dataverse&dvobject_types........

And it hits the condition - FacesContext.getCurrentInstance() == null in the DataverseLocaleBean.java , and set localecode = "en" . Any suggestions on this?

@mheppler
Copy link
Contributor

@JayanthyChengan after much hacking and a little hair pulling, I can say that I don't know what the problem was other than there was a problem with using h:inputHidden components to pass the bundle variables that was causing the errors with the Edit Account btn. Switching those to use a var in the <script> tag at the bottom of the page resolved those.

Since I spent so long investigating, and not getting anywhere, I started to reformat some of the code, so I just went ahead and made a pull request with the fix and my reformatting changes. Nothing major. Just used jQuery where I could, since most of the code is already using it. Take a look and let me know if that looks good.

@JayanthyChengan
Copy link
Contributor Author

@mheppler - Thank you for your investigation and the PR. I will test the code and let you know soon.

JayanthyChengan and others added 3 commits December 13, 2018 17:03
Fixed 'ResourceBundles are immutable' error on My Data [ref IQSS#4679]
…cript-bundle

# Conflicts:
#	src/main/webapp/dataset.xhtml
@pdurbin
Copy link
Member

pdurbin commented Mar 20, 2019

There are merge conflicts in Bundle_fr.properties but I believe that in the long term this file will be remove in favor of the version at https://github.com/GlobalDataverseCommunityConsortium/dataverse-language-packs

@JayanthyChengan
Copy link
Contributor Author

@mheppler - I tested the code which looks good for me. Can you please move it to codereview?

Thanks

@mheppler
Copy link
Contributor

mheppler commented Apr 3, 2019

Good stuff @JayanthyChengan. Thank you. The new var approach looks great. I have added a few minor suggestions to improve where the bundle text is located on the page, in relation to previously existing message text, as well as some other minor house cleaning of the loginpage that will tiddy up some long forgotten loose ends.

@JayanthyChengan
Copy link
Contributor Author

@mheppler - Actually, the new var approach was implemented by you in this pull request scholarsportal#29 and added the comment #5351 (comment). I merged and tested the code, it looks good for me.

Sorry, I am not clear what other suggestions you are pointing to?

src/main/java/Bundle.properties Outdated Show resolved Hide resolved
src/main/java/Bundle.properties Outdated Show resolved Hide resolved
src/main/java/Bundle.properties Outdated Show resolved Hide resolved
src/main/java/Bundle_fr.properties Outdated Show resolved Hide resolved
src/main/webapp/loginpage.xhtml Outdated Show resolved Hide resolved
@mheppler
Copy link
Contributor

mheppler commented Apr 4, 2019

You should be able to see my comments in the files changed view of this PR. It looks like they were "pending" though, as part of a review form, so that was on me for not hitting "submit". You should see them now.

@JayanthyChengan
Copy link
Contributor Author

@mheppler - please review the code. Thanks

@mheppler
Copy link
Contributor

mheppler commented Apr 4, 2019

@JayanthyChengan thanks for the quick turn around, but I've got two last items...

  • In dataverseuser.xhtml <f:loadBundle basename="Bundle" var="bundle"/> (line 26) should be removed, it just looks you resorted the initial change
  • In Bundle.properties and in Bundle_fr.properties, the to and of values (line 2209, 2210) are still hanging at the bottom, there is a section of general single words at the top of the bundles, ending at line 71, that they should get moved to

@JayanthyChengan
Copy link
Contributor Author

@mheppler - added new commits.

Copy link
Contributor

@mheppler mheppler left a comment

Choose a reason for hiding this comment

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

👍 Looks good, @JayanthyChengan. Thanks!

@kcondon kcondon merged commit edf1f6d into IQSS:develop Apr 5, 2019
@pdurbin pdurbin added this to the 4.13 milestone Apr 22, 2019
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

Successfully merging this pull request may close these issues.

7 participants