[#9493] Convert FieldValidator to a utility class #9498

VinodKW · 2019-02-25T09:45:26Z

Fixes #9493 Covert FieldValidator to a utility class Also modified all files which are related to FieldValidator class.

teammates-bot · 2019-02-25T09:45:27Z

Hi @VinodkumarWagh, these parts of your pull request do not appear to follow our contributing guidelines:

PR Title
- Issue Reference (#<issue-number>) missing.

RonakLakhotia · 2019-02-25T10:28:33Z

@VinodkumarWagh please look take care of the following.

Fix lint errors
See the build details to check your errors
Update your PR title

src/main/java/teammates/common/util/FieldValidator.java

amrut-prabhu

Make sure that your code compiles before you push changes to your PR.
The changes that you've made here are not sufficient and do not resolve the issue.

As mentioned in the issue description, there are 3 steps needed for FieldValidator:

all methods are static
the class is final
and the constructor is private
You will also need to fix other places that break once you make these changes.

src/main/java/teammates/common/util/FieldValidator.java

VinodKW · 2019-02-25T14:52:28Z

@RonakLakhotia @amrut-prabhu @wangsha Can anybody help me to solve that lint error? How do I resolve it? I'm only getting that one error, except that error all things are going fine.

rrtheonlyone · 2019-02-25T14:57:28Z

Hi @VinodkumarWagh, to fix the lint error you should refer to your TRAVIS report. This is the output:

[ant:checkstyle] [ERROR] /home/travis/build/TEAMMATES/teammates/src/main/java/teammates/common/util/FieldValidator.java:414: Line has trailing whitespaces. [RegexpSinglelineJava]
[ant:checkstyle] [ERROR] /home/travis/build/TEAMMATES/teammates/src/main/java/teammates/common/util/FieldValidator.java:416: 'METHOD_DEF' has more than 1 empty lines before. [EmptyLineSeparator]
[ant:checkstyle] [ERROR] /home/travis/build/TEAMMATES/teammates/src/main/java/teammates/common/util/FieldValidator.java:708: Line is longer than 125 characters (found 126). [LineLength]

In future, make sure to run ./gradlew lint and npm run lint locally before pushing in your code and creating a PR!

monmanuela · 2019-02-25T15:02:22Z

Just a nifty trick that may be useful in the future, you can run npm run lint:ts -- --fix to automatically fix some of the lint errors 😄

VinodKW · 2019-02-27T04:29:45Z

@amrut-prabhu I've tried each and every combination for the given error still not getting it, what could be wrong in it?
[ant:checkstyle] [ERROR] /home/vinod/teammates/src/main/java/teammates/common/util/FieldValidator.java:264:5: Static variable definition in wrong order. [DeclarationOrder]

amrut-prabhu · 2019-02-27T04:39:51Z

@VinodkumarWagh This may help.

See the order mentioned under

Checks that the parts of a class or interface declaration appear in the order suggested by the Code Conventions for the Java Programming Language.

VinodKW · 2019-02-27T04:48:47Z

@amrut-prabhu I've tried that too and also tried it again by implementing it according to the file that you had suggested to me(TimeHelper.java).

amrut-prabhu · 2019-02-27T04:55:59Z

Are you sure about that? The link says to place constructors after class variables

VinodKW · 2019-02-27T05:03:07Z

@amrut-prabhu got it, I apologize, btw thanks!

VinodKW

@amrut-prabhu Changes have made successfully.

amrut-prabhu

Good attempt at the updates!
But, try not to make unintended changes (like unnecessarily removing whitespace or pasting something by accident as mentioned in the first couple of review comments)

src/main/java/teammates/common/util/FieldValidator.java

amrut-prabhu · 2019-02-27T21:57:02Z

src/main/java/teammates/common/util/FieldValidator.java

@@ -142,7 +144,6 @@
    public static final String TEAM_NAME_IS_VALID_EMAIL_ERROR_MESSAGE =
            "The field " + TEAM_NAME_FIELD_NAME + " is not acceptable to TEAMMATES as the suggested value for "
                    + TEAM_NAME_FIELD_NAME + " can be mis-interpreted as an email.";
-


here too, and the rest of them as well

src/main/java/teammates/common/util/FieldValidator.java

amrut-prabhu · 2019-02-27T22:12:35Z

src/test/java/teammates/test/cases/util/FieldValidatorTest.java

    @Test
    public void testGetValidityInfoForNonHtmlField_cleanInput_returnEmptyString() {
        String clean = "Valid clean input with no special HTML characters";
        String testFieldName = "Inconsequential test field name";
-        String actual = validator.getValidityInfoForNonHtmlField(testFieldName, clean);
+        String actual = getValidityInfoForNonHtmlField(testFieldName, clean);


As mentioned above.

amrut-prabhu · 2019-02-27T22:13:49Z

src/main/java/teammates/common/datatransfer/attributes/FeedbackSessionAttributes.java

-        addNonEmptyError(validator.getValidityInfoForNonNullField(
-                FieldValidator.FEEDBACK_SESSION_NAME_FIELD_NAME, feedbackSessionName), errors);
+        addNonEmptyError(FieldValidator.getValidityInfoForNonNullField(
+                    FieldValidator.FEEDBACK_SESSION_NAME_FIELD_NAME, feedbackSessionName), errors);


Why the extra indent?

amrut-prabhu · 2019-02-27T22:15:17Z

src/main/java/teammates/common/datatransfer/attributes/FeedbackSessionAttributes.java


-        addNonEmptyError(validator.getValidityInfoForNonNullField(FieldValidator.COURSE_ID_FIELD_NAME, courseId), errors);
+        addNonEmptyError(FieldValidator.getValidityInfoForNonNullField(


It might be better to just move FieldValidator.getValidityInfoForNonNullField( to the next line as well

amrut-prabhu · 2019-02-27T22:16:00Z

src/main/java/teammates/common/datatransfer/attributes/FeedbackSessionAttributes.java


-        addNonEmptyError(validator.getValidityInfoForNonNullField("instructions to students", instructions), errors);
+        addNonEmptyError(FieldValidator.getValidityInfoForNonNullField(


amrut-prabhu · 2019-02-27T22:16:59Z

src/main/java/teammates/common/datatransfer/attributes/StudentAttributes.java

@@ -175,25 +175,23 @@ public boolean isEnrollInfoSameAs(StudentAttributes otherStudent) {
        // id is allowed to be null when the student is not registered
        Assumption.assertNotNull(team);
        Assumption.assertNotNull(comments);
-


Don't remove this whitespace

@amrut-prabhu Thanks for the review, I will surely correct them all.

VinodKW · 2019-02-28T16:01:11Z

@amrut-prabhu Why Travis-CI is not showing the report?

VinodKW

Ready to review.

wkurniawan07 · 2019-02-28T16:46:29Z

@VinodkumarWagh Until Travis CI gives green, it is not ready for review (unless you requested for partial review, which seems not to be the case). There are merge conflicts that you need to resolve.

VinodKW · 2019-02-28T16:53:54Z

@wkurniawan07 But it's been hours now for Travis CI to give the green light, is there any problem with it? Because I had push that changes hours ago.

wkurniawan07 · 2019-02-28T16:56:22Z

@VinodkumarWagh if there are merge conflicts, no CI would be able to run.

VinodKW · 2019-03-01T15:08:01Z

@wkurniawan07 I am creating a new pull request for this issue.

Updated

ca255ca

Merge branch 'master' into master

01a88aa

RonakLakhotia reviewed Feb 25, 2019

View reviewed changes

src/main/java/teammates/common/util/FieldValidator.java Outdated Show resolved Hide resolved

amrut-prabhu requested changes Feb 25, 2019

View reviewed changes

src/main/java/teammates/common/util/FieldValidator.java Show resolved Hide resolved

src/main/java/teammates/common/util/FieldValidator.java Show resolved Hide resolved

Update FieldValidator.java

344f6ef

VinodKW changed the title ~~Updated~~ [#9493] Convert FieldValidator to a utility class Feb 25, 2019

Declare all methods as static, class as final and constructor as private

afce2cc

VinodKW force-pushed the master branch from 344f6ef to afce2cc Compare February 25, 2019 14:13

VinodKW force-pushed the master branch from 8cfd3df to afce2cc Compare February 26, 2019 17:26

Changed all files related to FieldValidator

6fcda36

VinodKW force-pushed the master branch from 77d116a to 6fcda36 Compare February 27, 2019 02:53

Changes as per the FieldValidator

c363289

Additional modifications

a1cdfb4

VinodKW force-pushed the master branch from 0911fca to a1cdfb4 Compare February 27, 2019 05:36

VinodKW commented Feb 27, 2019

View reviewed changes

wkurniawan07 requested a review from amrut-prabhu February 27, 2019 11:44

wkurniawan07 added the s.ToReview The PR is waiting for review(s) label Feb 27, 2019

wkurniawan07 assigned amrut-prabhu Feb 27, 2019

amrut-prabhu requested changes Feb 27, 2019

View reviewed changes

Fixed Reviewed corrections

1d4e2c2

VinodKW force-pushed the master branch from a8b89b9 to 1d4e2c2 Compare February 28, 2019 03:19

VinodKW added 3 commits February 28, 2019 20:15

Fixed Reviewed Corrections

794d849

Reviewed Correction Fixed

fd1461c

Additional changes

e79bd33

VinodKW commented Feb 28, 2019

View reviewed changes

VinodKW added 2 commits February 28, 2019 23:20

removed conflicts

e19b6ee

resolved conflict

6e0611a

VinodKW closed this Mar 1, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#9493] Convert FieldValidator to a utility class #9498

[#9493] Convert FieldValidator to a utility class #9498

VinodKW commented Feb 25, 2019 •

edited

teammates-bot commented Feb 25, 2019

RonakLakhotia commented Feb 25, 2019

amrut-prabhu left a comment

VinodKW commented Feb 25, 2019

rrtheonlyone commented Feb 25, 2019

monmanuela commented Feb 25, 2019

VinodKW commented Feb 27, 2019

amrut-prabhu commented Feb 27, 2019

VinodKW commented Feb 27, 2019 •

edited

amrut-prabhu commented Feb 27, 2019

VinodKW commented Feb 27, 2019

VinodKW left a comment

amrut-prabhu left a comment •

edited

amrut-prabhu Feb 27, 2019

amrut-prabhu Feb 27, 2019

amrut-prabhu Feb 27, 2019

amrut-prabhu Feb 27, 2019

amrut-prabhu Feb 27, 2019

amrut-prabhu Feb 27, 2019

VinodKW Feb 28, 2019

VinodKW commented Feb 28, 2019

VinodKW left a comment

wkurniawan07 commented Feb 28, 2019

VinodKW commented Feb 28, 2019

wkurniawan07 commented Feb 28, 2019

VinodKW commented Mar 1, 2019


		addNonEmptyError(validator.getValidityInfoForNonNullField(FieldValidator.COURSE_ID_FIELD_NAME, courseId), errors);
		addNonEmptyError(FieldValidator.getValidityInfoForNonNullField(

[#9493] Convert FieldValidator to a utility class #9498

[#9493] Convert FieldValidator to a utility class #9498

Conversation

VinodKW commented Feb 25, 2019 • edited

teammates-bot commented Feb 25, 2019

RonakLakhotia commented Feb 25, 2019

amrut-prabhu left a comment

Choose a reason for hiding this comment

VinodKW commented Feb 25, 2019

rrtheonlyone commented Feb 25, 2019

monmanuela commented Feb 25, 2019

VinodKW commented Feb 27, 2019

amrut-prabhu commented Feb 27, 2019

VinodKW commented Feb 27, 2019 • edited

amrut-prabhu commented Feb 27, 2019

VinodKW commented Feb 27, 2019

VinodKW left a comment

Choose a reason for hiding this comment

amrut-prabhu left a comment • edited

Choose a reason for hiding this comment

amrut-prabhu Feb 27, 2019

Choose a reason for hiding this comment

amrut-prabhu Feb 27, 2019

Choose a reason for hiding this comment

amrut-prabhu Feb 27, 2019

Choose a reason for hiding this comment

amrut-prabhu Feb 27, 2019

Choose a reason for hiding this comment

amrut-prabhu Feb 27, 2019

Choose a reason for hiding this comment

amrut-prabhu Feb 27, 2019

Choose a reason for hiding this comment

VinodKW Feb 28, 2019

Choose a reason for hiding this comment

VinodKW commented Feb 28, 2019

VinodKW left a comment

Choose a reason for hiding this comment

wkurniawan07 commented Feb 28, 2019

VinodKW commented Feb 28, 2019

wkurniawan07 commented Feb 28, 2019

VinodKW commented Mar 1, 2019

VinodKW commented Feb 25, 2019 •

edited

VinodKW commented Feb 27, 2019 •

edited

amrut-prabhu left a comment •

edited