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

JBPM-7913 - Create pastebin workitem #76

Merged
merged 1 commit into from Nov 8, 2018
Merged

Conversation

tsurdilo
Copy link
Member

@tsurdilo tsurdilo commented Nov 5, 2018

No description provided.

@tsurdilo
Copy link
Member Author

tsurdilo commented Nov 5, 2018

pastebin workitem (can create a new pastebin)

@tsurdilo tsurdilo changed the title pastebin workitem JBPM-7913 - Create pastebin workitem Nov 5, 2018
@tsurdilo tsurdilo force-pushed the pastebin branch 2 times, most recently from 2b72d1f to 7856477 Compare November 5, 2018 21:15
public class CreatePastebinWorkitemHandler extends AbstractLogOrThrowWorkItemHandler {

private String develKey;
private PastebinPaste pastebin;
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need to have pastebin as field? It makes it not really thread safe...

Copy link
Member Author

Choose a reason for hiding this comment

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

changed

workItem);

String title = (String) workItem.getParameter("Title");
Document contentDoc = (Document) workItem.getParameter("Content");
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess simple text would make sense too, maybe have to arguments Content and ContentDoc or something like that, wdyt?


<artifactId>pastebin-workitem</artifactId>
<name>Pastebin</name>
<description>Create new or get existing pastes from Pastebin</description>
Copy link
Contributor

Choose a reason for hiding this comment

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

I couldn't find the work item handler to get existing pastes... is this still planned to be implemented?

Copy link
Member Author

Choose a reason for hiding this comment

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

Wanted to and would be easy but for scraping existing padtebin makes you buy the pro version which costs money, so i deferred it for now.can add

Copy link
Member Author

Choose a reason for hiding this comment

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

added

@tsurdilo
Copy link
Member Author

tsurdilo commented Nov 6, 2018

@mswiderski updated + added get existing pastebin handler

@tsurdilo
Copy link
Member Author

tsurdilo commented Nov 6, 2018

Jenkins retest please

String author = (String) workItem.getParameter("Author");

// for testing
PastebinPaste testPastebin = (PastebinPaste) workItem.getParameter("TestPastebin");
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe replace this (and the below if else) with following

PastebinPaste pastebin = (PastebinPaste) workItem.getParameter("Pastebin");
if (pastebin == null) {
    pastebin = new PastebinPaste();
}

that way it's not really test specific as users can simply add their pastebins manually. Moreover you can even skip all other parameters if the pastebin is given...

Copy link
Member Author

Choose a reason for hiding this comment

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

done


String pastebinKey = (String) workItem.getParameter("PastebinKey");
// for testing
PastebinLink testPastebinLink = (PastebinLink) workItem.getParameter("TestPastebinLink");
Copy link
Contributor

Choose a reason for hiding this comment

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

similar here, allow to provide PastebinLink as work item param so it won't be test specific

Copy link
Member Author

Choose a reason for hiding this comment

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

done here too. thanks.

@tsurdilo
Copy link
Member Author

tsurdilo commented Nov 7, 2018

@mswiderski updated - thanks!

@tsurdilo
Copy link
Member Author

tsurdilo commented Nov 7, 2018

rebased and updated pom to 7.15.0-SNAPSHOT. should build fine now.

@tsurdilo
Copy link
Member Author

tsurdilo commented Nov 7, 2018

Jenkins retest please

1 similar comment
@tsurdilo
Copy link
Member Author

tsurdilo commented Nov 7, 2018

Jenkins retest please

@tsurdilo tsurdilo merged commit 44e7b52 into kiegroup:master Nov 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants