Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JDF 149 - shows how to use DeltaSpike BeanManagerProvider to access CDI in a EntityListener #365

Closed
wants to merge 8 commits into from

Conversation

rafabene
Copy link
Contributor

No description provided.

@rafabene
Copy link
Contributor Author

This is ready for review

Technologies: CDI, Deltaspike, JPA, JSF
Summary: Shows how to use DeltaSpike BeanManagerProvider to access CDI in a EntityListener
Prerequisites:
Target Product:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

per @pmuir, target product is WFK

@sgilda
Copy link
Contributor

sgilda commented Nov 29, 2012

@rafabene : Very nice! Tested this and it works great! I made a few minor suggestions for the README file.

I would like to see instructions on the rendered application page, similar to the comments in the README.

  • To insert a new contact, click the New Contact button, complete the form fields, and then click the Save button. Notice that both the All Contacts and Audit Records tables are updated.
  • To modify an existing contact, find the name in the All Contacts table and click the Select for edit button. Modify the data and then click the Save button. Notice that the Audit Records table is updated with the new contact data.,
  • To remove a contact, find the name in the All Contacts table and click the Remove button. Notice that the Audit Records table is updated with the new contact data.,

But I'm afraid it might take up to much real estate on the page.

Looks good to me!


Create, edit or remove some contacts to see the audit records being registered.

_NOTE: To fire the update audit, you must change one of the contact fields
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't render properly. It's missing the ending underscore, :-)
NOTE: To fire the update audit, you must change one of the contact fields

@sgilda
Copy link
Contributor

sgilda commented Nov 29, 2012

@pmuir: This is ready for your technical review :-).


This project demonstrates the use of DeltaSpike's BeanManagerProvider.

BeanmanagerProvider provides access to the BeanManager by registering the current BeanManager during the startup. This is really handy if you like to access CDI functionality from places where no CDI based injection is available. If a simple but manual bean-lookup is needed, it's easier to use the BeanProvider.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The last sentence is a bit confusing. Maybe "If a simple but manual bean-lookup is needed, it's easier to use BeanProvider instead"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, you should use backticks to enclose references to classes e.g. BeanProvider

@pmuir
Copy link
Contributor

pmuir commented Nov 30, 2012

looks good, but it's very short on javadoc and comments. We need to increase this. Pretend you are writing this as a real app, as well as a tutorial.

@rafabene
Copy link
Contributor Author

rafabene commented Dec 3, 2012

I think that's better now :)

@@ -0,0 +1,95 @@
jboss-as-deltaspike-beanmanager: Shows how to use DeltaSpike BeanManagerProvider to access CDI in a EntityListener
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The quickstart name doesn't match the quickstart folder name. This name should probably be deltaspike-beanmanagerprovider to match the folder name. Also, the "jboss-as" prefix is only used in the URL to access the application. :-)

@sgilda
Copy link
Contributor

sgilda commented Dec 20, 2012

Ohter than changing the quickstart name in the first line of the README.md file from jboss-as-deltaspike-beanmanager to deltaspike-beanmanagerprovider to match the folder name, this looks good to me!

@pmuir , do you need to review this?


/**
* This method will promote the conversation when this component is constructed through the {@link PostConstruct} annotation
* This will also create a new entity to be managed/edited
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where do you end the conversation?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually this is never ended since the user has no option to leave the screen :0

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should add a comment to that effect. Explain what is going on there.

@pmuir
Copy link
Contributor

pmuir commented Dec 22, 2012

Pretty short on comments still. Every file, every public method needs at least one javadoc comment explaining what it is for. Put comments about the implementation into normal comments.

@sgilda
Copy link
Contributor

sgilda commented Jan 2, 2013

The instructions are a big help!

I'm being a little picky here, but it's hard to distinguish button and table names in the instructions for the running application. Is it possible to use quotes or italics for the form title and button names in the contactsCRUD.xhtml like this:

  • To insert a new contact: Click the New Contact button, complete the form fields, and then click the Save button. Notice that both the All Contacts and Audit Records tables are updated.
  • To modify an existing contact: Find the name in the All Contacts table and click the Select for edit button. Modify the data and then click the Save button. Notice that the Audit Records table is updated with the new contact data.
  • To remove a contact: Find the name in the All Contacts table and click the Remove button. Notice that the Audit Records table is updated with the new contact data.

I wonder if it would look better with the form fields and everything else left aligned. It looks a little odd to me now with the Contacts title, result message, form fields, and *Note centered, while the rest is left aligned.

@sgilda
Copy link
Contributor

sgilda commented Jan 15, 2013

Looks good.
Pete, are you OK with this one now? If so, I will merge.

@pmuir
Copy link
Contributor

pmuir commented Jan 16, 2013

Looks good.

@sgilda
Copy link
Contributor

sgilda commented Jan 16, 2013

I get conflicts when I rebase upstream/master:
$ git rebase upstream/master
First, rewinding head to replay your work on top of it...
Applying: Initial commit
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging README.md
CONFLICT (add/add): Merge conflict in README.md
Auto-merging .gitignore
CONFLICT (add/add): Merge conflict in .gitignore
Failed to merge in the changes.
Patch failed at 0001 Initial commit
The copy of the patch that failed is found in:
/home/sgilda/GitRepos/quickstart-jdf/.git/rebase-apply/patch

For some reason, the README.md file has this at the end:
+deltaspike-quickstarts
+======================
+
+Holds some Deltaspike quickstarts

The .gitignore also has errors.

@rafabene : this could be due to your initial commit. It looks like you check in a root README.md file and a .gitignore file. These are messing it up. :-)

@sgilda
Copy link
Contributor

sgilda commented Jan 16, 2013

Rebased upstream, squashed the commits, and merged.

@sgilda sgilda closed this Jan 16, 2013
@rafabene rafabene deleted the JDF-149 branch January 29, 2013 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants