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

(Chore) Refactors and redundant code removal #1454

Merged
merged 6 commits into from
Mar 25, 2024
Merged

(Chore) Refactors and redundant code removal #1454

merged 6 commits into from
Mar 25, 2024

Conversation

lozette
Copy link
Collaborator

@lozette lozette commented Mar 22, 2024

Changes

Some refactoring and removal of unneeded code, plus a small bugfix.

  • Replace uses of FactoryBot create with build where possible to speed things up
  • Replace multiple uses of mocks where only one is needed
  • Remove a redundant migration that was used once only
  • Remove an unused task and its skipped tests
  • Fix a small bug in the Project presenter, the true value of a Transfer task was being obscured by some inappropriate shared code

@lozette lozette force-pushed the chore/refactors branch 3 times, most recently from 2bd0dc3 to ec254c8 Compare March 22, 2024 15:20
@lozette lozette changed the title Chore/refactors (Chore) Refactors and redundant code removal Mar 22, 2024
@lozette lozette marked this pull request as ready for review March 22, 2024 15:28
Speed things up slightly by only calling ProjectsForExportService once
This was a task designed to be run once to update historic projects which did
not have a mandatory piece of data (the date the Grant Certificate was received)

As we do not need this importer now, we can delete it to speed up the test suite
This task is not used in the task list and all its tests are skipped. Remove it
for now - we can re-add it if it's needed later.
…possible

Slim down these specs by removing repeated unnecessary calls to mocks, and
using `build` instead of `create` for stubs where appropriate.
These specs currently iterate over 20-odd pages to check for the absence of
a button. We only need to check one page, as all the task pages use the same
Policy to restrict edit access. This speeds up two of our slowest specs.
We were using the same method `not_applicable_for_a_transfer` method to check
the correct value was being output for both Academy order type and Sponsored
grant type for transfers. However, the method `not_applicable_for_a_transfer`
only checked the Academy order type, so it was concealing a potential bug in
the Sponsored grant type presenter.

Remove this method and explicitly check for the correct values.
Copy link

sonarcloud bot commented Mar 22, 2024

Copy link
Collaborator

@mec mec left a comment

Choose a reason for hiding this comment

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

👩‍🍳 👌 🚢

@lozette lozette merged commit 5b051cc into main Mar 25, 2024
7 checks passed
@lozette lozette deleted the chore/refactors branch March 25, 2024 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants