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

Refactor controllers out of folders #368

Merged
merged 4 commits into from
Aug 18, 2023

Conversation

Cruikshanks
Copy link
Member

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

This change is not directly related to WATER-4092. This is more of a technical debt exercise. We were about to embark on a spike of seeing if we could replicate what the legacy update billing account address code is doing. Repeating what we already have would mean creating a new folder to put the single controller file in.

This seems to be needlessly complicating the controllers' folder. It's not something we have done in other areas which highlights we are pre-empting future needs. Certainly, when a routes controller file becomes too unwieldy we will most likely break it down into separate controllers grouped in a folder.

But let's wait until we need to do that! 😁

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

This change is not directly related to WATER-4092 . This is more a technical debt exercise. We were about to embark on a spike of seeing if we could replicate what the legacy update billing account address code is doing. Repeating what we already have would mean creating a new folder to put the a single controller file in.

This seems to be needlessly complicating the controllers folder. It's not something we have done in other areas which highlights we are pre-empting future needs. Certainly, when a routes controller file becomes to unwieldy we will most likely break it down into separate controllers grouped in a folder.

But lets wait until we need to do that! 😁
@Cruikshanks Cruikshanks added the housekeeping Refactoring, tidying up or other work which supports the project label Aug 18, 2023
@Cruikshanks Cruikshanks self-assigned this Aug 18, 2023
The `DataController` is an example of what we are currently doing. Most of `/health` was migrated from another project. Based on how we have built `DataController` and what we have just done with the other controllers it makes sense to refactor 'health'.
@Cruikshanks Cruikshanks marked this pull request as ready for review August 18, 2023 12:32
@Cruikshanks Cruikshanks requested review from Jozzey and StuAA78 August 18, 2023 12:32
Copy link
Contributor

@Jozzey Jozzey left a comment

Choose a reason for hiding this comment

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

@Cruikshanks Cruikshanks merged commit 0b0bf91 into main Aug 18, 2023
@Cruikshanks Cruikshanks deleted the housekeeping-refactor-controller-folder branch August 18, 2023 13:55
Cruikshanks added a commit that referenced this pull request Aug 29, 2023
https://eaflood.atlassian.net/browse/WATER-4092

WATER-4092 will require us to add a new 'validator'. These are the modules that contain [Joi](https://joi.dev/) schemas we use to check inbound request payloads.

Whilst working on [Add new billing account change address validator](#383) we spotted a few things we wanted to change on the existing validator. As we only have the one, now seemed a good time to make the changes.

- fix the test filename (we'd omitted `.test`)
- move it out of the `/bill-runs` folder

On that last one, similar to [Refactor controllers out of folders](#368) if we find we have lots of validators in the future we'll likely move them into folders. But right now it feels pre-emptive.
Cruikshanks added a commit that referenced this pull request Aug 29, 2023
https://eaflood.atlassian.net/browse/WATER-4092

WATER-4092 will require us to add a new 'validator'. These modules contain [Joi](https://joi.dev/) schemas we use to check inbound request payloads.

Whilst working on [Add new billing account change address validator](#383) we spotted a few things we wanted to change on the existing validator. As we only have one, now seemed a good time to make the changes.

- fix the test filename (we'd omitted `.test`)
- move it out of the `/bill-runs` folder

On that last one, similar to [Refactor controllers out of folders](#368) if we find we have lots of validators in the future we'll likely move them into folders. But right now it feels pre-emptive.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
housekeeping Refactoring, tidying up or other work which supports the project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants