search

GENESIIS / campus

0 stars 0 forks source link

C13-public : view course page : display all the relevent details for a course #13

Open Chamindana opened 7 years ago

Chamindana commented 7 years ago

Business benefit: Display course details content. Implement rating facility to a course.

Estimation: Dev - 32sp Testing - 32sp

chathuriM commented 7 years ago

201610241506 CM

chathuriM commented 7 years ago

201610241644 CM

chathuriM commented 7 years ago

201610242224 CM

chathuriM commented 7 years ago

201610250912 CM

chathuriM commented 7 years ago

programmeview

chathuriM commented 7 years ago

201610251430 CM

chathuriM commented 7 years ago

201610251435 CM

chathuriM commented 7 years ago

201610252300 CM

chathuriM commented 7 years ago

201610261619 CM

chathuriM commented 7 years ago

201610272128 CM

chathuriM commented 7 years ago

201610281016 CM

chathuriM commented 7 years ago

201610281018 CM

chathuriM commented 7 years ago

201610281100 CM

c13-Display course details- Modified execute method and add method comments 45a02d8 c13-Display course details- Modified execute method sql query for retrieve 799b772 c13-Display course details-Removed unwanted values in dql query-cm 6090405 c13-Display course details-Modified execute() and add method comments cda4436 c13-Display course details-Modified execute method, retrive semester code 2812d86 c13-Display course details-Modified jsp page for display data-cm 988d900

chathuriM commented 7 years ago

201611011553 CM

chathuriM commented 7 years ago

201611011555 CM.

chathuriM commented 7 years ago

201611011650 CM

chathuriM commented 7 years ago

201611022204 CM

chathuriM commented 7 years ago

201611021313 CM

chathuriM commented 7 years ago

201611021313 CM

-c13-display course details- Moved to RFQA.

HimashaFernando commented 7 years ago

201611031440 HF

QA comments

  1. Course provider name should be linked to it's mini web, not to it's web site.
  2. There can be two columns for store the tutor of modules in the module table, 'tutored by' and 'tutor'. Currently, course detail view refer only 'tutored by' column. But, it should be changed to refer those two columns and display the relevant value. Which value should be displayed is depend on another attribute flag. This attribute is not in DDL_v9. @Miyuru-Genesiis will add this to the db and update the dml. After that 'displaying the tutor details for module' should be refactored to refer 'tutored by' and 'tutor' both columns according to the situation.
  3. 'Display program location' function has not been implemented in this issue. Clarifications needed on this function from marketing. Is it needed to display the program location in program detail view and how should it be displayed? FYA @tharinduw
  4. Program artwork/image displaying function should be implemented using system configuration. It should be refactored as per stand up discussion.
  5. Class type value has repeated in the UI 'Find More' section.
  6. Empty program details are displayed in the UI when empty strings retrieved from data base. Ex; 'Entry Requirements', 'Intakes', 'Counselor Name', etc. The way of handling this situation should be discussed and it should be considered when this issue is integrated with the UI,
  7. Displaying emails should be linked with 'Mail-To' function.

Note: Only a basic testing on the main use case, display program details was carried out in this phase since the UI is not ready yet and data inserting function has not implemented yet.

Assigning back to @chathuriM to consider above comments.

chathuriM commented 7 years ago

201611081452 CM

Due to the design change that's intended to support both JSTL and JSON data , through the same CampusController.java (servlet) and the decision to follow the multi-page Application approach instead of SPA (Single Page Application), the rest of the code progression will be conducted in c13-display-course-details-MP-cm branch.

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

chathuriM commented 7 years ago

201611081551 CM

chathuriM commented 7 years ago

201611101421 CM

chathuriM commented 7 years ago

201611111252 CM

chathuriM commented 7 years ago

201611111527 CM

chathuriM commented 7 years ago

201611151140 CM

QA Modifications Done: 201611031440 HF QA comments

  1. Course provider linked to a mini web and also ones off providers not linked to any web site.
  2. Displayed tutors according to new DDL
  3. Programme town and district implemented
  4. Programme images displayed using campus.xml configuration
  5. QA Review "Class type value has repeated in the UI 'Find More' section" Class type value has not repeated. Please check it using different values for class type and description.
  6. As instructed by TW empty strings will be handled on Data adding Module.
  7. As TW Instructed emails not linked to 'Mail to Function'. Because separate program and institute inquiry functions are there.

C13-Display Course details Moved to Code review.

pabodhaW commented 7 years ago

201611151235 PN Code review comments Improvements

  1. ClassTypeDAO.java class logger initialization is wrong. Mistakenly developer has given a wrong class name. static Logger log = Logger.getLogger(ModuleDAO.class.getName()); change the class name into ClassTypeDAO.java.
  2. Inside DAO class there is a String instance for assign the query. But again, while setting it into the preparestatement, developer convert again into a String. `String query = "SELECT p.NAME, t.NAME , d.NAME FROM CAMPUS.PROGRAMME p inner join CAMPUS .PROGRAMMETOWN g "
    • "on p.CODE=g.PROGRAMME inner join CAMPUS.TOWN t on t.CODE=g.town inner join CAMPUS.DISTRICT d on t.DISTRICT=d.CODE where p.CODE=? and g.ISACTIVE=1"; preparedStatement = conn.prepareStatement(query.toString());` I think this is not necessary. KTD.
  3. When taking a data from ResultSet manage your own pattern. I prefer giving the column name rather than the number. This is not a must. Just to improve readability. rs.getString(2) over rs.getString("CODE").
  4. Remove unwanted variables declared. (In ProgrammeDAO.findById(Object code)() ) method. ) Line 76: HashMap<String, Object> hashmap = new HashMap<String, Object>();
  5. calculateYears(), calculateMonth(), calculateWeeks() etc. I prefer if developer can move those methods into a separate class which can be reuse by another developer.
  6. The above mentioned methods, does not handle the exceptions properly. Log them and throw. public void calculateMonth(int totalDays) { try { // assumes all months have 30 days months = (int) totalDays / 30; totalDays %= 30; calculateWeeks(totalDays); } catch (Exception e) { // TODO: handle exception } }
  7. Take CmdViewProgramme.calculateRating() method into another class that can be reuse in a different issue.
  8. In JSTL code consider Open Close Principle. When there is a a requirement to extend the code, minimize the effort that you have to do into the JSP page. (This does not require any code change. Just to improve the code maintainability.)

@chathuriM FYA.

chathuriM commented 7 years ago

201611152327 CM

chathuriM commented 7 years ago

201611152314 CM

chathuriM commented 7 years ago

201611161047 CM

chathuriM commented 7 years ago

201611161123 CM

chathuriM commented 7 years ago

201611161440 CM

chathuriM commented 7 years ago

201611161502 CM Code modifications after crev.

rts 201611151235 PN

When taking a data from ResultSet manage your own pattern. I prefer giving the column name rather than the number. This is not a must. Just to improve readability. rs.getString(2) over rs.getString("CODE").

Above improvement not implement here because I used SQL inner joins.

Take CmdViewProgramme.calculateRating() method into another class that can be reuse in a different issue.

As instructed by TW Rating is future implementation. So This method is not moved to another class.

@pabodhaW assigning this back to you to do the verification.

chathuriM commented 7 years ago

201611161511 CM

What has been done(Scope)

List all details relevant to program

Above mentioned details are displayed in jsp page for get details about courses to users.

Change/Add Files $ git diff --name-status d776aa1a90c40f68aab0b87f6c3d3a20339d12f3..4035804b6c380 458ca4fcafc3cff0038a883ed1a

Grade of risk of change High

Sprint released Sprint 1

Unit Test Done

How can it be tested Browse : localhost:8080 or www.campus.dev

What will it affect To the man preview of browsing course details.

How can it be tested Click on View ProgrammeDetails Button on Home Page

What user roles are affected public user/student

Other comments Use Number 1 as a Program ID.

anuradha999 commented 7 years ago

201611151235 AS Code review issue fixes

rtc 201611151235 PN

Other than 3 and 7 all the other mentioned issues are fixed. rtc 201611161502 CM

In 7th issue will be fix in future implementation. I'm moving this issue to the next stage, QA.

chathuriM commented 7 years ago

201611241110 CM

c13-display-couurse-details merged with c-v1.0-sp1 branch

chathuriM commented 7 years ago

201611241155 CM

Programme rating system implemented in this issue. But As Instructed by @tharinduw I've commented both front end and backend code. Rating Algorithm added to CmdViewProgramme.java class. Please refer the code if you need it in future implementations.

Chamindana commented 7 years ago

201611291630 CN- Local testing summary

1.Footer image is not set .The following error is displayed.

campus9

2.Additional cage is going to display in chrome browser.

campus10

3.The following error is identified when loading the page.

ERROR [com.genesiis.campus.command.CmdViewProgramme] (http-0.0.0.0:8080-5) calculateRating() : ejava.lang.NumberFormatException: For input string: "?" 12:07:27,885 ERROR [com.genesiis.campus.command.CmdViewProgramme] (http-0.0.0.0:8080-5) execute() : ejava.lang.NumberFormatException: For input string: "?" 12:07:27,885 INFO [com.genesiis.campus.util.DataHelper] (http-0.0.0.0:8080-5) getResultView() : java.lang.NumberFormatException: For input string: "?" 12:07:27,887 ERROR [com.genesiis.campus.controller.CampusController] (http-0.0.0.0:8080-5) process(): Exception : java.lang.NumberFormatException: For input string: "?"

Assign back to @chinthakacw for MX fix.

chinthakacw commented 7 years ago

201611301530 CW c13-display-course-details-MP-c13 issue solving done-cw c95d52bae0

chinthakacw commented 7 years ago

201611302150 CW

QA Modifications Done: 201611291630 CN

1.Footer image is not set .The following error is displayed. 2.Additional cage is going to display in chrome browser.

ERROR [com.genesiis.campus.command.CmdViewProgramme] (http-0.0.0.0:8080-5) calculateRating() : ejava.lang.NumberFormatException: For input string: "?" 12:07:27,885 ERROR [com.genesiis.campus.command.CmdViewProgramme] (http-0.0.0.0:8080-5) execute() : ejava.lang.NumberFormatException: For input string: "?" 12:07:27,885 INFO [com.genesiis.campus.util.DataHelper] (http-0.0.0.0:8080-5) getResultView() : java.lang.NumberFormatException: For input string: "?" 12:07:27,887 ERROR [com.genesiis.campus.controller.CampusController] (http-0.0.0.0:8080-5) process(): Exception : java.lang.NumberFormatException: For input string: "?"

C13-Display Course details Moved to Code review.

pabodhaW commented 7 years ago

201612010956 PN Code review comments Improvements

  1. CmdViewProgramme.calculateRating() code has changed by developer. But not mentioned it in header comments. Also not explained what has done to the code..

  2. code-details.jsp page has changed by developer. No header comments.

Assigning this issue back to @chinthakacw

chinthakacw commented 7 years ago

201612011303 CW c13-display-course-details-MP-crev modifications done - cw 2714f55d6c

chinthakacw commented 7 years ago

201612011314CW

-c13-display course details- Moved to QA.

Chamindana commented 7 years ago

201612011715 CN -Local test summary

rtc 201611151140 CM

All fixed.

rtc 201611302150 CW

Error 1, 2, 3, All fixed. Tested and verified.

The following point is identified.

1.The error message is displayed when loading the Image.

courseview

Assigning back to @chinthakacw to consider the point.