CLDR-16838 Login bug; preliminary refactoring, clean-up; phase BETA not deprecated #3386
Conversation
…ot deprecated -Delete dead code and fix compiler warnings in WebContext.java, SurveyMain.java, and related files -Remove the @deprecated label for the phase BETA since we still use it -Delete the other deprecated phases and dependent code -Add warning comments for methods that appear unused but are still accessed by jsp -Comments
| */ | ||
| @Deprecated | ||
| BETA("Beta", CheckCLDR.Phase.SUBMISSION); |
There was a problem hiding this comment.
I started to delete BETA along with the other deprecated items, then noticed that we did use BETA at the start of v44, and there's a lot of dependent code on both the front and back ends. Unlike the other deprecated items, which have caused a lot of confusion due to their overlapping/redundant meanings, BETA seems like it may still be useful and isn't likely to be used accidentally. It shouldn't be deprecated unless we actually intend to remove it.
| if (CookieSession.tooManyObservers()) { | ||
| if (session != null) { | ||
| // allow in administrator or TC. | ||
| if (!UserRegistry.userIsTC(user)) { |
There was a problem hiding this comment.
this is an auto-refactor by IntelliJ, to fix a warning about empty "if" block -- it comes out looking more complicated than it really is
There was a problem hiding this comment.
makes sense. Though, It'd be easier to read if the refactor was a separate PR or at least a separate commit.
There was a problem hiding this comment.
Oh, this IS the refactor PR. I reread it. It looks like it has some wording cleanup too, which is fine. LGTM!
| if (CookieSession.tooManyObservers()) { | ||
| if (session != null) { | ||
| // allow in administrator or TC. | ||
| if (!UserRegistry.userIsTC(user)) { |
There was a problem hiding this comment.
makes sense. Though, It'd be easier to read if the refactor was a separate PR or at least a separate commit.
| if (CookieSession.tooManyObservers()) { | ||
| if (session != null) { | ||
| // allow in administrator or TC. | ||
| if (!UserRegistry.userIsTC(user)) { |
There was a problem hiding this comment.
Oh, this IS the refactor PR. I reread it. It looks like it has some wording cleanup too, which is fine. LGTM!
-Delete dead code and fix compiler warnings in WebContext.java, SurveyMain.java, and related files
-Remove the Deprecated label for the phase BETA since we still use it
-Delete the other deprecated phases and dependent code
-Add warning comments for methods that appear unused but are still accessed by jsp
-Comments
CLDR-16838
ALLOW_MANY_COMMITS=true