Skip to content

Comments

6214 user notification following ingest#6271

Merged
kcondon merged 4 commits intoIQSS:developfrom
CIMMYT:6214-User-Notification-Following-Ingest
Oct 18, 2019
Merged

6214 user notification following ingest#6271
kcondon merged 4 commits intoIQSS:developfrom
CIMMYT:6214-User-Notification-Following-Ingest

Conversation

@alejandratenorio
Copy link
Contributor

@alejandratenorio alejandratenorio commented Oct 11, 2019

New Contributors

Welcome! New contributors should at least glance at CONTRIBUTING.md, especially the section on pull requests where we encourage you to reach out to other developers before you start coding. Also, please note that we measure code coverage and prefer you write unit tests. Pull requests can still be reviewed without tests or completion of the checklist outlined below. Note that we use the "closes" syntax below to trigger Github's automation to close the corresponding issue once the pull request is merged.

Thanks for your contribution to Dataverse!

Related Issues

Pull Request Checklist

@djbrooke
Copy link
Contributor

Thanks @alejandratenorio @ricvazo!

Moving my question here from #6214... does this provide messaging for ingest failures or only successes?

@alejandratenorio
Copy link
Contributor Author

@djbrooke,

An email is always sent, if the ingest fails a message is sent: (Error) with datafileName (src/main/java/edu/harvard/iq/dataverse/ingest/IngestMessageBean.java line 109 ).
Maybe we should add the message to Bundle.properties, what do you think?

@coveralls
Copy link

coveralls commented Oct 11, 2019

Coverage Status

Coverage decreased (-0.01%) to 19.462% when pulling 857ab97 on CIMMYT:6214-User-Notification-Following-Ingest into 8991919 on IQSS:develop.

@djbrooke
Copy link
Contributor

djbrooke commented Oct 11, 2019

Hi @alejandratenorio thanks for the quick response. If you can add that to bundle.properties it would be great, since that will allow others to translate the message. I'll be able to edit the text there, so just add any text that you feel makes sense. I need to:

I'll also assign @landreev to this issue so that we can discuss next week. We have a long weekend but we'll back in the office on Tuesday.

@alejandratenorio
Copy link
Contributor Author

@djbrooke, yeah, we'll add it. Thank you and regards.

@alejandratenorio
Copy link
Contributor Author

Hi @djbrooke, @ricvazo changed messages when an ingest fails.

@djbrooke
Copy link
Contributor

Thanks @ricvazo @alejandratenorio

@landreev - all yours. Let me know when you've completed code review and I'll update the bundle text.

@landreev landreev assigned djbrooke and unassigned landreev Oct 17, 2019
@djbrooke djbrooke removed their assignment Oct 18, 2019
@kcondon kcondon self-assigned this Oct 18, 2019
@kcondon kcondon merged commit 4539927 into IQSS:develop Oct 18, 2019
@djbrooke djbrooke added this to the 4.18 milestone Oct 21, 2019
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.

User Notification Following Ingest

6 participants