Bug 461624 - Generate index.html in default Dynamic Web Project
Summary: Generate index.html in default Dynamic Web Project
Status: ASSIGNED
Alias: None
Product: WTP Java EE Tools
Classification: WebTools
Component: jst.j2ee (show other bugs)
Version: unspecified   Edit
Hardware: PC Linux
: P3 enhancement (vote)
Target Milestone: 3.7.1   Edit
Assignee: Daniel Azarov CLA
QA Contact: Chuck Bridgham CLA
URL:
Whiteboard: greatfix_candidate
Keywords: helpwanted
Depends on:
Blocks:
 
Reported: 2015-03-06 16:53 EST by Daniel Azarov CLA
Modified: 2015-08-20 12:47 EDT (History)
5 users (show)

See Also:
shr31223: review+
shr31223: review? (cbridgha)


Attachments
Result of accessing the application (665.76 KB, image/png)
2015-03-06 16:53 EST, Daniel Azarov CLA
no flags Details
Proposal wizard change (33.20 KB, image/png)
2015-03-06 16:53 EST, Daniel Azarov CLA
no flags Details
Patch (16.33 KB, patch)
2015-03-20 13:47 EDT, Daniel Azarov CLA
no flags Details | Diff
Patch (9.32 KB, patch)
2015-04-02 13:51 EDT, Daniel Azarov CLA
no flags Details | Diff
Patch (11.11 KB, patch)
2015-04-29 14:13 EDT, Daniel Azarov CLA
daniel: review?
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Azarov CLA 2015-03-06 16:53:12 EST
Created attachment 251374 [details]
Result of accessing the application

Default Dynamic Web Project does not contain index.html and so running it on a server always require to generate this file first. That's a very typical scenario and should be fixed for a more seamless experience.

Accessing the application at http://localhost:8080/helloworld/ shows "Forbidden" as shown in the attachment forbidden.png.

It could be a check box "Generate index.html" see attachment "GenerateIndexHtml.png"
Comment 1 Daniel Azarov CLA 2015-03-06 16:53:58 EST
Created attachment 251375 [details]
Proposal wizard change
Comment 2 Roberto Sanchez Herrera CLA 2015-03-12 12:59:45 EDT
Good enhancement. Targeting initially to WTP 3.7, and adding helpwanted keyword. 

A patch is welcome :)
Comment 3 Daniel Azarov CLA 2015-03-20 13:47:05 EDT
Created attachment 251795 [details]
Patch
Comment 4 Daniel Azarov CLA 2015-03-20 13:50:44 EDT
probably it is no need to add index.html generation in AppClientFacetInstallDelegate.createFlexibleProject
Comment 5 Daniel Azarov CLA 2015-03-31 13:56:04 EDT
Roberto, could you review my patch, please?
Comment 6 Roberto Sanchez Herrera CLA 2015-04-01 11:59:24 EDT
(In reply to Daniel Azarov from comment #5)
> Roberto, could you review my patch, please?

Hi Daniel, 
Thank you for the patch. I quickly reviewed it, and noticed a couple of issues:
1. The code to create the checkbox for creating index.html was added to J2EEModuleFacetInstallPage. But this change is only for web projects, so I suggest adding it to WebFacetInstallPage instead.

2. Same comment for changes in IJ2EEFacetInstallDataModelProperties and J2EEFacetInstallDataModelProvider, I think IWebFacetInstallDataModelProperties and WebFacetInstallDataModelProvider should be used instead. 

3. Not sure why the patch includes changes in Application client module classes, but I do not think these classes should change. Maybe the changes can be avoided by moving the code to the classes I suggest in 1 and 2. 

Those are the comment I have so far
Comment 7 Daniel Azarov CLA 2015-04-02 13:51:27 EDT
Created attachment 252148 [details]
Patch
Comment 8 Daniel Azarov CLA 2015-04-02 13:53:22 EDT
Roberto, thank you for all your comments,
could you review my new patch?
Comment 9 Roberto Sanchez Herrera CLA 2015-04-29 11:48:18 EDT
(In reply to Daniel Azarov from comment #8)
> Roberto, thank you for all your comments,
> could you review my new patch?

Hi, 
Thank you for the new patch, and sorry for the delay for reviewing it again. 
I have some more comments after reviewing the latest patch:

- Please update copyright header in java files: if the header has only one year (like Copyright (c) 2003 IBM Corporation and others.), add the second year (like Copyright (c) 2003, 2015 IBM Corporation and others.). If it already has 2 years, update the second to be 2015)
 
- In IWebFacetInstallDataModelProperties, the new property is:
public static final String GENERATE_INDEX_HTML = "IJ2EEFacetInstallDataModelProperties.GENERATE_INDEX_HTML";
I suggest changing "IJ2EEFacetInstallDataModelProperties.GENERATE_INDEX_HTML" to "IWebFacetInstallDataModelProperties.GENERATE_INDEX_HTML"

- In WebFacetInstallDelegate, why not generating an HTML 5 file? Just courious. I'm OK with what you propose.

- In JavaEEPreferencesInitializer, the new preference is called APPLICATION_GENERATE_INDEX_HTML. Given that this is related to web projects only, I suggest using  something like DYNAMIC_WEB_GENERATE_INDEX_HTML or WEB_GENERATE_INDEX_HTML, because the APPLICATION prefix is more related to EAR projects

- In JavaEEPreferencesInitializer, remove the preference APP_CLIENT_GENERATE_INDEX_HTML. I think is not used/needed

- In WebFacetInstallPage.properties, the new message should have the & character, to check/uncheck it when the user presses Alt, like the "Generate web.xml deployment descriptor" label. It could be "Generate &index.html"
Comment 10 Daniel Azarov CLA 2015-04-29 14:13:36 EDT
Created attachment 252909 [details]
Patch

Roberto, thank you for your comments,
could you review my new patch?
Comment 11 Roberto Sanchez Herrera CLA 2015-04-30 11:27:37 EDT
(In reply to Daniel Azarov from comment #10)
> Created attachment 252909 [details]
> Patch
> 
> Roberto, thank you for your comments,
> could you review my new patch?

Thank you, the patch looks good. 

I'm asking Chuck's review. Chuck, this includes changes in UI, so if approved, would need PMC approval. 
Also, the 3.7 M7 build is happening this week (today I believe), so, lets see if this can be included in 3.7 or will need to wait until next release.
Comment 12 Victor Rubezhny CLA 2015-08-20 12:47:02 EDT
Chuck, any movement on the issue?