-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add copy from existing returns #1056
Conversation
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.
Looks really good, just a few comments :)
app/services/return-requirements/fetch-return-requirements.service.js
Outdated
Show resolved
Hide resolved
app/services/return-requirements/fetch-return-requirements.service.js
Outdated
Show resolved
Hide resolved
app/services/return-requirements/fetch-return-requirements.service.js
Outdated
Show resolved
Hide resolved
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.
Few more small comments :)
@@ -5,23 +5,25 @@ | |||
* @module SetupService | |||
*/ | |||
|
|||
const FetchReturnRequirementsService = require('./fetch-return-requirements.service.js') | |||
const SetupPresenter = require('../../presenters/return-requirements/setup.presenter.js') | |||
const SessionModel = require('../../models/session.model.js') |
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.
Just a nit comment but these two need to be swapped round
const SessionModel = require('../../models/session.model.js')
const SetupPresenter = require('../../presenters/return-requirements/setup.presenter.js')
// Thing under test | ||
const ReturnRequirementModel = require('../../app/models/return-requirement.model.js') | ||
|
||
describe('Return version model', () => { |
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.
describe('Return version model', () => { | |
describe('Return requirement model', () => { |
'use strict' | ||
|
||
/** | ||
* @module ReturnVersionHelper |
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.
* @module ReturnVersionHelper | |
* @module ReturnRequirementHelper |
* | ||
* @param {Object} [data] Any data you want to use instead of the defaults used here or in the database | ||
* | ||
* @returns {Promise<module:ReturnVersionModel>} The instance of the newly created record |
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.
* @returns {Promise<module:ReturnVersionModel>} The instance of the newly created record | |
* @returns {Promise<module:ReturnRequirementModel>} The instance of the newly created record |
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.
https://eaflood.atlassian.net/browse/WATER-4355
As part of our work to replace NALD for handing return requirements we want to be able to copy data from an existing returns. This PR adds the option to the "How do you want to set up the returns" page and checks to see if there is any existing returns for the current licence. If there is show the extra option.