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

AP 663 Proceed to substantive application #593

Merged
merged 7 commits into from
Jun 21, 2019

Conversation

reggieb
Copy link
Contributor

@reggieb reggieb commented Jun 19, 2019

AP-663

Main change

Adds a page to the providers journey (after used_delegated_functions if the provider selects "Yes") that allows the provider to choose between continuing with a substantive application, or returning later.

Knock on change

If the provider chooses to return later they are prompted with a date in the future by which they should complete the application. That date is "the current day plus 10 working days". To work that out I needed to:

  • Add a system to calculate working days (WorkingDayCalculator). This uses the business gem
  • To accommodate changes in bank holiday and to avoid having to rely on the gems hardcoded list of bank holidays, they are retrieved from a Gov-UK api by BankHolidayRetriever and stored in a model BankHoliday
  • A worker (BankHolidayUpdateWorker) is triggered on calling BankHoliday.dates that:
    • Checks to see if there is a current store of bank holiday data (current being less than 2 days old) and closes without doing anything if there is
    • Gets the latest data from the API and if the data is the same as the currently stored version, just updates the updated_at of the BankHoliday instance holding that data.
    • If the data is different to the current data, it creates a new BankHoliday instance to store the data

@reggieb reggieb added the ready for review Please review label Jun 19, 2019
Copy link
Contributor

@stephenrichards stephenrichards left a comment

Choose a reason for hiding this comment

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

Nice work on the Bank holidays. Would be an ideal candidate for a gem.


def self.dates
BankHolidayUpdateWorker.perform_in 10.seconds
instance = by_updated_at.last || create
Copy link
Contributor

Choose a reason for hiding this comment

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

create! with an exclamation mark?
😃

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Damn! I was so pleased with myself for using save! in the worker. I even did a quick "Check for non-bang save methods before Julien's spots them" check.

end

def save
super && model.provider_used_delegated_functions!
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a bit uneasy by putting the change of state here because I think that for all the other states it's done in the controller.
So for consistency sake, it would be better to follow the same pattern.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And it would mean the overloading of save would no longer be necessary. Nice one.

def dates(group)
return if data.empty?

data.dig(group, 'events')&.map { |details| details['date'] }
Copy link
Contributor

Choose a reason for hiding this comment

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

would .pluck('date') work instead of .map { |details| details['date'] } ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes - it might well do.

@reggieb reggieb added UAT and removed ready for review Please review labels Jun 20, 2019
@reggieb reggieb force-pushed the ap-663-proceed-to-substantive-application branch from d0d73c2 to 11c86c8 Compare June 20, 2019 11:23
@reggieb reggieb force-pushed the ap-663-proceed-to-substantive-application branch from 11c86c8 to ae130bb Compare June 21, 2019 15:18
@reggieb reggieb merged commit 5ed6231 into master Jun 21, 2019
@reggieb reggieb deleted the ap-663-proceed-to-substantive-application branch June 21, 2019 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants