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
Display error when retrieving resource and refactors load and save actions. #1508
Conversation
24e046c
to
d13dd8a
Compare
LANGUAGE_SELECTED: setMyItemsLang, | ||
SHOW_RDF_PREVIEW: showRdfPreview, | ||
LANGUAGES_RECEIVED: languagesReceived, | ||
LOADING_LANGUAGES: loadingLanguages, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should change these to RETRIEVE_LANGUAGES_STARTED/FINISHED
for consistency.
@@ -26,7 +26,7 @@ describe('fetchResourceTemplate', () => { | |||
await fetchResourceTemplate(resourceTemplateId, dispatch) | |||
expect(dispatch).toBeCalledWith({ type: 'RETRIEVE_RESOURCE_TEMPLATE_STARTED', payload: resourceTemplateId }) | |||
expect(dispatch).toBeCalledWith({ | |||
type: 'RETRIEVE_ERROR', | |||
type: 'SET_RETRIEVE_RESOURCE_TEMPLATE_ERROR', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like SET_*
for events. The events should say what happened, not how to handle it. I prefer RETRIEVE_RESOURCE_TEMPLATE_ERROR
for this.
src/actions/index.js
Outdated
type: 'APPEND_RESOURCE', | ||
payload: { reduxPath, resource, resourceTemplates }, | ||
export const setResourceTemplate = resourceTemplate => ({ | ||
type: 'SET_RESOURCE_TEMPLATE', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe RESOURCE_TEMPLATE_LOADED
? Not SET_
src/actions/index.js
Outdated
type: 'TOGGLE_COLLAPSE', | ||
payload: { reduxPath }, | ||
export const setResourceTemplateSummary = resourceTemplateSummary => ({ | ||
type: 'SET_RESOURCE_TEMPLATE_SUMMARY', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe RESOURCE_TEMPLATE_SUMMARY_LOADED
? Not SET_
src/actions/index.js
Outdated
export const clearSearchResults = () => ({ | ||
type: 'CLEAR_SEARCH_RESULTS', | ||
export const setSaveResourceTemplateError = (resourceTemplateId, reason) => ({ | ||
type: 'SET_SAVE_RESOURCE_TEMPLATE_ERROR', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SAVE_RESOURCE_TEMPLATE_ERROR
not SET_
src/actions/index.js
Outdated
export const clearResourceTemplates = () => ({ | ||
type: 'CLEAR_RESOURCE_TEMPLATES', | ||
export const setSaveResourceError = (uri, reason) => ({ | ||
type: 'SET_SAVE_RESOURCE_ERROR', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SAVE_RESOURCE_ERROR
not SET_
… Refactors save/load resource/resource template actions for consistency. closes #1447
d13dd8a
to
abc10ae
Compare
Various renaming completed @jcoyne . |
No description provided.