SAK-45827 - gradebook classic jsf upgrade

maurercw commented 2 years ago

If/when this ever gets merged, some points of emphasis for testing:

Everywhere dates and times are used for display, as well as date pickers. Formats and timezones especially.
All Grades tab - this table got a massive overhaul and I'm hopeful that it works. Sorting is a bit wonky from time to time, especially if you have empty values. Not really sure what the story is with that.

ottenhoff commented 2 years ago

IS someone willing to test this? It looks like a lot of effort! We don't use GB Classic much at all but this change looks solid .....

ottenhoff commented 2 years ago

@austin48 are you willing to test this? You seem to be the only one still running gb1

austin48 commented 2 years ago

@ottenhoff , Yes I can take a look at it. It's quite a big change, so it could take some time. We have some customizations to the All Grades tab, so I'll have to see if this affects our changes.

Also, the JIRA number doesn't seem to match the title.

maurercw commented 2 years ago

Well, dang, did I do all those commits under the wrong jira? Should be SAK-45872

austin48 commented 2 years ago

@ottenhoff and @maurercw, I just realized that this patch only applies to Sakai 22+. (haha, after I tried applying but failed to compile this patch for our Sakai 21.x instance, I think, due to errors on methods not supported in 21).

I can try testing on a "vanilla" 22 instance, but it will be a bummer that I won't be able to test with our customizations for a while (I'm not scheduled to start upgrading to 22 until early 2023).

maurercw commented 2 years ago

Pretty sure I did it against 21x, but I'd have to go back and confirm since it's been a while!

Chris

On Tue, May 24, 2022, 5:55 PM austin48 @.***> wrote:

@ottenhoff https://github.com/ottenhoff and @maurercw https://github.com/maurercw, I just realized that this patch only applies to Sakai 22+. (haha, after I tried applying but failed to compile this patch for our Sakai 21.x instance, I think, due to errors on methods not supported in 21).

I can try testing on a "vanilla" 22 instance, but it will be a bummer that I won't be able to test with our customizations for a while (I'm not scheduled to start upgrading to 22 until early 2023).

— Reply to this email directly, view it on GitHub https://github.com/sakaicontrib/gradebook1/pull/20#issuecomment-1136468021, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACBPNZ47E3YE5QCMGD62RZ3VLVF63ANCNFSM5A7CZJBA . You are receiving this because you were mentioned.Message ID: @.***>

austin48 commented 2 years ago

i'm seeing compile errors on 21.x like

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:compile (default-compile) on project sakai-gradebook-app-ui: Compilation failure: Compilation failure: 
[ERROR] /sakai-src/gradebook1/app/ui/src/java/org/sakaiproject/tool/gradebook/ui/RosterBean.java:[578,45] cannot find symbol
[ERROR]   symbol:   method getELContext()
[ERROR]   location: variable facesContext of type javax.faces.context.FacesContext
[ERROR] /sakai-src/gradebook1/app/ui/src/java/org/sakaiproject/tool/gradebook/ui/RosterBean.java:[577,53] cannot find symbol
[ERROR]   symbol:   method getExpressionFactory()
[INFO] Could not resolve module specifier "rollup-plugin-node-resolve"
[ERROR]   location: class javax.faces.application.Application
[INFO] Could not resolve module specifier "rollup-plugin-commonjs"
[ERROR] /sakai-src/gradebook1/app/ui/src/java/org/sakaiproject/tool/gradebook/ui/RosterBean.java:[584,45] cannot find symbol
[ERROR]   symbol:   method getELContext()
[ERROR]   location: variable facesContext of type javax.faces.context.FacesContext
[ERROR] /sakai-src/gradebook1/app/ui/src/java/org/sakaiproject/tool/gradebook/ui/RosterBean.java:[583,53] cannot find symbol
[ERROR]   symbol:   method getExpressionFactory()
[ERROR]   location: class javax.faces.application.Application
[ERROR] /sakai-src/gradebook1/app/ui/src/java/org/sakaiproject/tool/gradebook/ui/RosterBean.java:[612,33] cannot find symbol
[ERROR]   symbol:   method setValueExpression(java.lang.String,javax.el.ValueExpression)
[ERROR]   location: variable dynamicDataTable of type org.apache.myfaces.component.html.ext.HtmlDataTable
[ERROR] /sakai-src/gradebook1/app/ui/src/java/org/sakaiproject/tool/gradebook/ui/RosterBean.java:[651,21] cannot find symbol
[ERROR]   symbol:   method setValueExpression(java.lang.String,javax.el.ValueExpression)
[ERROR]   location: variable link of type javax.faces.component.html.HtmlCommandLink
[ERROR] /sakai-src/gradebook1/app/ui/src/java/org/sakaiproject/tool/gradebook/ui/RosterBean.java:[653,21] cannot find symbol
[ERROR]   symbol:   method setActionExpression(javax.el.MethodExpression)
[ERROR]   location: variable link of type javax.faces.component.html.HtmlCommandLink
[ERROR] /sakai-src/gradebook1/app/ui/src/java/org/sakaiproject/tool/gradebook/ui/RosterBean.java:[685,23] cannot find symbol
[ERROR]   symbol:   method setValueExpression(java.lang.String,javax.el.ValueExpression)
[ERROR]   location: variable output of type javax.faces.component.html.HtmlOutputText
[ERROR] -> [Help 1]
[ERROR]

or did I not apply the patches correctly?

austin48 commented 2 years ago

I think the errors I mentioned above are due to our custom code in gradebook1. I can't reproduce the error when compiling a "vanilla" version of the gradebook1 21.x branch with this patch into our custom Sakai 21.x. Short of removing our custom code piece by piece (I tried removing a few places in our custom code that looked like it was using jsf faces context) until I find the problem, anyone have any guesses?

austin48 commented 2 years ago

I fixed our build error, sort of... There was a custom dependency that I think was no longer being used, but I don't know why it would make this fail (maybe it used an older version of jsf?) so I commented it out and the build completed. However, now on start up, I'm getting many errors like:

27-May-2022 13:47:24.329 ERROR [main] org.apache.commons.digester.Digester.error Parse Error at line 2 column 14: Document is invalid: no grammar found.
org.xml.sax.SAXParseException; systemId: jar:file:/tomcat/webapps/sakai-gradebook-tool/WEB-INF/lib/weld-servlet-shaded-3.1.3.Final.jar!/META-INF/faces-config.xml; lineNumber: 2; columnNumber: 14; Document is invalid: no grammar found.

27-May-2022 13:47:24.331 ERROR [main] org.apache.commons.digester.Digester.error Parse Error at line 2 column 14: Document root element "faces-config", must match DOCTYPE root "null".
org.xml.sax.SAXParseException; systemId: jar:file:/tomcat/webapps/sakai-gradebook-tool/WEB-INF/lib/weld-servlet-shaded-3.1.3.Final.jar!/META-INF/faces-config.xml; lineNumber: 2; columnNumber: 14; Document root element "faces-config", must match DOCTYPE root "null".

It looks like this PR's patch contains many changes to faces-config's xmlns. Again, it works fine on a vanilla Sakai install, but there must be more incompatibilities with our custom code... maybe more dependencies that use old JSF? do we also have to update those dependencies to use jsf2? I'd hate to keep playing whack-a-mole trying to merge this patch into our custom code.

sakaicontrib / gradebook1

SAK-45827 - gradebook classic jsf upgrade #20