search

GENESIIS / campus

0 stars 0 forks source link

C36-tutor: add tutor information : save tutor profile details #34

Open chathuriM opened 7 years ago

chathuriM commented 7 years ago

Business benefit : A tutor can register in campus.lk as a lecturer/tutor and use his/her created profile to log into the system so that he/she can interact with the system to view course details.

Tasks:

UI pages : Tutor registration page

Estimation: UI - 64sp Dev - 64sp Testing - 32sp

chathuriM commented 7 years ago

201611181130 CM

Mokeup design for tutor profile

2016_11_18_11_19_31-page-001

chathuriM commented 7 years ago

201611211200 CM INIT c36-add-tutor-details-cm branch. (Branch off from c13-display-course-details-MP-cm)

chathuriM commented 7 years ago

201611211223 CM

chathuriM commented 7 years ago

201611211448 CM

chathuriM commented 7 years ago

201611211657 CM

chathuriM commented 7 years ago

201611221011 CM

chathuriM commented 7 years ago

2016112221657 CM

chathuriM commented 7 years ago

201611231350 CM

chathuriM commented 7 years ago

201611231559 CM

chathuriM commented 7 years ago

201611251623 CM

c36- Add Tutor Details Moved to Code review.

Miyuru-Genesiis commented 7 years ago

201611281300 CM - Code Review undertaken by MM



2. Remove redundant `finally` block. 

* CmdCheckUsername.java - 
The potential that `tutor.setUsername(helper.getParameter("USERNAME"));` may return `null` has not been handled. This has the potential to produce problematic behaviour in DAO.

* CmdAddTutorProfile.java - 
1. The unneeded, commented-out lines are better to be removed. 
2. Exception type thrown is better to be mentioned in logger statements.

* CountryDAO.java - 
1. The exception type that is thrown is not mentioned in logger statement
2. Missing JavaDoc comment

* TownDAO.java - 
Missing JavaDoc comment

* TutorDAO.java -  

1.`add()` method: 
- [ ] Use the now-available `ApplicationStatus` enum to set values for columns such as `ISACTIVE`. 
- [ ] Better to move the statement `final Tutor tutor = (Tutor) object;` within the `try `block because of the risk of a `ClassCasteException`
- [ ] The exception type that is thrown is not mentioned in logger statement

2. `findById()` method: 
- [ ] Better to move the statement `final Tutor tutor = (Tutor) object;` within the `try `block because of the risk of a `ClassCasteException`
- [ ] The exception type that is thrown is not mentioned in logger statement
- [ ] JavaDoc not present

* Tutor.java
1. Make identifier `MySpace` lowerCamelCase (`mySpace`) to conform to naming conventions.
2. Better to change field names to be more meaningful. E.g., instead of simply `viber`, this could be `viberNumber`. Personally, i feel this applies to fields `facebook`, `linkedIn`, `twitter`, `instagram`, `MySpace`, `whatsApp` other than `viber`.

* PublicController.java, PublicCmdFacotry.java - 
These files do not seem to be utilised in the issue. The are, therefore, better be removed to minimise merge conflicts. 

* Validator.java -
Missing JavaDoc comment for `calculateYears(String)`

* addTutorDetails.jsp - 
Better to place addTutorDetails.jsp file inside `dist\partials\` directory to adhere to the common practice followed in other issues.

**Assigning back to the developer to address the above points in MX.**
chinthakacw commented 7 years ago

201611281749 CW c36-add-tutor-details-working on c36 code review issue solving - cw 3777de9fc5

chinthakacw commented 7 years ago

201611291504 CW c36-add-tutor-details - code review issue solving done in the branch of c36-add-tutor-details-cw 142612f89d

jayani1991 commented 7 years ago

201612011148 -JH Code review comments

/**
     * Calculate number of years in the Duration
     * 
     * @author Chathuri
     * @param duration
     * @return void
     */

  1. Log the error in catch statement with the word Exception in it. Ex: instead of this

    catch (Exception exception) {
        log.error("calculateYears:  e" + e.toString());
        throw e;
    }

    Use log.error(“calculateYears: Exception “ + exception.toString());

  2. In calculateMonths(),calculateWeeks(), and calculateDates() methods it returens an int value. But in method comments it says “@return void”

-CmdAddToturDetails.java

  1. execute() method 

                tutor.setLandAreaCode("0");
    tutor.setMobileNetworkCode("0");

    There are static values assigned to some of tutor attributes. Explain the code. It static values are needed then give reasons.

  2. setVariables(IDataHelper helper) The method name in the method and the comments are not equal. Check the name and the method comments.

@chinthakacw assigning this to you back to fix the coding issues.

chinthakacw commented 7 years ago

201612011747 - CW

chinthakacw commented 7 years ago

201612020951 CW New modifications in the c36-add-tutor-details

Estimation:

chinthakacw commented 7 years ago

201612042207 CW c36-add-tutor-details-Removing the tutor image path from the coding done - cw db46c65b8e

chinthakacw commented 7 years ago

201612190849 CW c36-add-tutor-details-loading town codes according to Country done - cw 0272aaa700

chinthakacw commented 7 years ago

201612191305 CW Modify the code and the classes to add details to the Country code, Area code and the phone number fields separately done - cw e191cdc41d