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

Add View License Returns page #967

Merged
merged 40 commits into from
May 3, 2024
Merged

Add View License Returns page #967

merged 40 commits into from
May 3, 2024

Conversation

jonathangoulding
Copy link
Collaborator

@jonathangoulding jonathangoulding commented May 1, 2024

https://eaflood.atlassian.net/browse/WATER-4444

The existing service handling view license is slow because it loads all the data for the tabs in one render. Work has been done previously to refactor the summary page to load only the summary information.

This change will introduce a returns controller, service and presenter to handle the view license returns page.

This will share the same view as the summary page and load the same 'common data' established in previous work.

jonathangoulding and others added 8 commits May 1, 2024 15:47
The existing service handling view license is slow because it loads all the data for the tabs in one render. Work has been done previously to refactor the summary page to load only the summary information.

This change will introduce a returns controller, service and presenter to handle the view license returns page.

This will share he same view as the summary page and load the same 'common data' established in previous work.

Both pieces of work are part of - https://eaflood.atlassian.net/browse/WATER-4444

Previous work as mentioned above - #957
get data from returns table

format data to presenter

update tests
add description to table row items
@jonathangoulding
Copy link
Collaborator Author

Need to implement the no returns tab / view.

@jonathangoulding jonathangoulding marked this pull request as ready for review May 3, 2024 15:22
app/routes/licence.routes.js Outdated Show resolved Hide resolved
app/routes/licence.routes.js Outdated Show resolved Hide resolved
app/presenters/licences/view-licence-returns.presenter.js Outdated Show resolved Hide resolved
Copy link
Member

@Cruikshanks Cruikshanks left a comment

Choose a reason for hiding this comment

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

They are nearly all just nit-picks which hopefully soon ESLint will save me from highlighting (or more often missing!)

I've also seen @robertparkinson got in there before me and I concur with what he is saying.

app/controllers/licences.controller.js Outdated Show resolved Hide resolved
app/controllers/licences.controller.js Outdated Show resolved Hide resolved
app/presenters/licences/view-licence-returns.presenter.js Outdated Show resolved Hide resolved
app/presenters/licences/view-licence-returns.presenter.js Outdated Show resolved Hide resolved
app/presenters/licences/view-licence-returns.presenter.js Outdated Show resolved Hide resolved
Copy link
Member

@Cruikshanks Cruikshanks left a comment

Choose a reason for hiding this comment

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

Some further tweaks. Went to check on SonarCloud. Happy to silence the method length stuff but it caught some things I should have spotted on the first pass.

app/presenters/licences/view-licence-returns.presenter.js Outdated Show resolved Hide resolved
app/presenters/licences/view-licence-returns.presenter.js Outdated Show resolved Hide resolved
app/presenters/licences/view-licence-returns.presenter.js Outdated Show resolved Hide resolved
jonathangoulding and others added 2 commits May 3, 2024 17:20
@jonathangoulding jonathangoulding force-pushed the view-licence-returns-page branch from 8eb5c00 to b895c74 Compare May 3, 2024 16:20
Co-authored-by: Alan Cruikshanks <[email protected]>
@Cruikshanks Cruikshanks merged commit 16caae5 into main May 3, 2024
6 checks passed
@Cruikshanks Cruikshanks deleted the view-licence-returns-page branch May 3, 2024 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants