Ticket #608 (closed planned_task: obsolete)

Opened 11 years ago

Last modified 10 years ago

BOOK_HTML_EXPORT_R0

Reported by: Astea Owned by: peko
Priority: 3 Milestone: M05_PRE5
Component: BOOK_MANAGING Version: 2.0
Keywords: Cc:
Category: EXTRA Effort: 0.5
Importance: 0 Ticket_group:
Estimated Number of Hours: Add Hours to Ticket:
Billable?: Total Hours:
Analysis_owners: dido,dido Design_owners: peko
Imp._owners: peko Test_owners:
Analysis_reviewers: deyan,deyan Changelog:
Design_reviewers: meddle, meddle Imp._reviewers: meddle, meddle
Test_reviewers: Analysis_score: 3
Design_score: 3 Imp._score: 3.5
Test_score: 0

Description

wiki page: BOOK_HTML_EXPORT_R0 - effort: 0.5d

Change History

comment:1 Changed 11 years ago by mitex

  • Status changed from new to s1a_analysis_started
  • Analysis_score set to 0
  • Test_score set to 0
  • Design_score set to 0
  • Owner changed from Astea to mitex
  • Imp._score set to 0
  • Analysis_owners set to mitex

changed owner

comment:2 Changed 10 years ago by mitex

  • Status changed from s1a_analysis_started to new
  • Analysis_owners mitex deleted

comment:3 Changed 10 years ago by dido

  • Owner changed from mitex to dido
  • Status changed from new to s1a_analysis_started
  • Analysis_owners set to dido

comment:4 Changed 10 years ago by dido

  • Status changed from s1a_analysis_started to s1b_analysis_finished

Currently there is a implementation of html export that could fit in this task, but it's not part of the trunk. It will be nice if we could reuse as much as possible of this.
Analysis (1h)

comment:5 Changed 10 years ago by deyan

  • Status changed from s1b_analysis_finished to new
  • Analysis_score changed from 0 to 2.5

A requirement for PDF export is

  • PDF export must be selected from menu File -> Export As... -> PDF

However, this was not implemented that way. I think this should be a requirement for this task as we don't want to have all of the export types in the menu. A submenu "Export As..." should be implemented. You should also mention what the limitations of the HTML export will be (kind of frames, rotated frames, audio, page borders, etc.)

comment:6 Changed 10 years ago by deyan

  • Analysis_reviewers set to deyan

comment:7 Changed 10 years ago by dido

  • Status changed from new to s1a_analysis_started

comment:8 Changed 10 years ago by dido

  • Status changed from s1a_analysis_started to s1b_analysis_finished

Updated according to the comments and test requirements added.
Analysis (1h)

comment:9 Changed 10 years ago by deyan

  • Status changed from s1b_analysis_finished to s1c_analysis_ok
  • Analysis_reviewers changed from deyan to deyan,deyan
  • Analysis_owners changed from dido to dido,dido
  • Analysis_score changed from 2.5 to 3

Please document well what is exported correctly, what is not, and what are losses.

comment:10 Changed 10 years ago by peko

  • Design_owners set to peko
  • Owner changed from dido to peko
  • Status changed from s1c_analysis_ok to s2a_design_started

comment:11 Changed 10 years ago by peko

  • Status changed from s2a_design_started to s2b_design_finished

comment:12 Changed 10 years ago by meddle

  • Status changed from s2b_design_finished to s1c_analysis_ok
  • Design_score changed from 0 to 2
  • Design_reviewers set to meddle
  • Code related to the design problems:
    • You didn't add the new html module to the main pom.xml!
    • The code related to the design MUST be merge-able with the trunk! You have some errors, so it is not.
    • About the test:
      • Use the FileEntryManager to get your files!
      • @Befor/@After/@Test annotations are missing, see GoodCodeExamples.
      • "book.pages().get().get(0).get(Page.class);" that's ugly, what do you think about this : "book.currentPage().get().get(Page.class, book);"?
      • Whats that in the test -> "e.printStackTrace();" when catching an exception, use the loger and write "fail("message");".
      • Why you wrote "new String(htmlString)". If you want to see if two strings without the white spaces are equal use the String#replaceAll("
        s", "") method.
    • Some constant things like the ".html" in your logic MUST be extracted to constants!
    • It is good to write assertations like "assert index > -1;", but write them messages too.
    • If the design code comes with new module, MAKE SURE it builds with maven and it runs with the true reader and true author runners!
  • Problems with the design:
    • Now we have "export to PDF" file->menu item, you must remve it and add only "export..." item which will contain the both "export to PDF" and "export to html" items, see the analysis requirements.
    • Write in the analysis, if not in the design your new shortcuts.
    • If you will keep all the html persisters in the extra.html module, you must write why, because the Persisters from the Persistence module are divided into their accurate modules, for example the ImagePersister is in the image module.
    • Write why you can not use the Persisters like Book->Storage from the Persistence module, the documentation must point why you can not reuse something already done. Write the differences in your use case.
    • One UML diagram should be in help, but it's not mandatory.

2p (75m)

comment:13 Changed 10 years ago by peko

  • Status changed from s1c_analysis_ok to s2a_design_started

comment:14 Changed 10 years ago by peko

  • Status changed from s2a_design_started to s2b_design_finished
  • Fixes and comments about the code:
    • Thanks for the comments! :-) Everything fixed as proposed except for:
      • The FileEntryManager is not suitable for test in my opinion. I do not use already created files and stuff, but rather create a file read it and then delete it. Correct me if I am wrong.
      • "book.pages().get().get(0).get(Page.class);" is not ugly, but is the right way to get a page from the model. What you propose is something to be announced as null and void and that had already been discussed. Same applies to page.currentFrame... What will happen if we have several applications dealing with one book(collaboration). Current things will become part of the view. These two methods should have already been deprecated but are not :-/.
      • About the exception, the logger has a method fatal("..."), not fail, but I suppose a better thing is to wrap and throw the exception. FileNotFoundException speaks better for itself instead of a message.
      • I do not see anything bad in new String(htmlString) and I do note get the idea of that comment. The "new String(htmlString)" has nothing to do with spaces. It creates a String from a StringBuilder. Replacing the white spaces as proposed will even require more resources instead of comparing the strings with the white spaces.
  • Fixes and comments about the design:
    • Menus fixed. Titles, mnemonics and stuff explained in the design. This includes the changes around menus that are not part of this task.
    • Persisters and things about them fixed in the design section. I do not think that the purpose of the new persisters should be expained. After all generally speaking persisters are defined by their schema. If the schema differs there is no need to explain why a persister cannot be reused. Small explanation given though.
    • Diagram will not be added, since I do not find it useful for the current number of persisiters. Maybe on later revision when we have more persisters a diagram will be useful.
  • Thanks for the comments. Maybe we should discuss some of the things for which we have different opinion.

comment:15 Changed 10 years ago by meddle

  • Status changed from s2b_design_finished to s2c_design_ok
  • Design_score changed from 2 to 3
  • Design_reviewers changed from meddle to meddle, meddle

The design is fine now, but why are you giving me sync-trunk revisions?? They of course are not merge-able with the trunk, you had to give me code that do not cause errors with the trunk... I fixed it... You have big problem with putting spacings it's a random matter...

1h (50m)

comment:16 Changed 10 years ago by meddle

:D I'm sleepy... 3p (50m) :D:D

comment:17 Changed 10 years ago by pap

I want to note that the menu should no be "Export as..." but "Export as". The "..." is used to mark that when the item is pressed a modal dialog will appear.

comment:18 Changed 10 years ago by peko

That is something I did not know :-)

comment:19 Changed 10 years ago by peko

  • Status changed from s2c_design_ok to s3a_implementation_started

comment:20 Changed 10 years ago by peko

  • Imp._owners set to peko

comment:21 Changed 10 years ago by peko

  • Status changed from s3a_implementation_started to s3b_implementation_finished

comment:22 Changed 10 years ago by meddle

  • Status changed from s3b_implementation_finished to s2c_design_ok
  • Imp._score changed from 0 to 2.5
  • Imp._reviewers set to meddle

The implementation fails because:

  • The test don't pass. The reason is there is no contents for the frames... Make sure that the test tests both image and text contents and then give it to review it again!
  • You have bad spacing (again), but that is not a reason to fail the implementation.
  • Bad way of getting a resource from it's resource ref:
    HotTextBookResource textResource = 
    	hotTextContent.mainResource().get().get(
    		HotTextBookResource.class);
    
    Use the frame parent of the content, if the path is ralative the above thing may not work...
  • Your Persisters can only save, so assert that other mode is impossible... Defensive programming man...

2.5p (35m)

comment:23 Changed 10 years ago by peko

  • Status changed from s2c_design_ok to s3a_implementation_started

comment:24 Changed 10 years ago by peko

  • Status changed from s3a_implementation_started to s3b_implementation_finished

Thanks the comments... Everything done as proposed. :-)

comment:25 Changed 10 years ago by meddle

  • Status changed from s3b_implementation_finished to s3c_implementation_ok
  • Imp._score changed from 2.5 to 3.5
  • Imp._reviewers changed from meddle to meddle, meddle

The implementation is ok now :)

3.5p (80m)

comment:26 Changed 10 years ago by deyan

  • Status changed from s3c_implementation_ok to closed
  • Resolution set to obsolete

Batch update from file query-obsoleted.csv

Note: See TracTickets for help on using tickets.