Skip to content
This repository has been archived by the owner on Aug 16, 2024. It is now read-only.

feat: Filter out unused irfo permit types from dropdown #190

Closed
wants to merge 1 commit into from

Conversation

fibble
Copy link
Contributor

@fibble fibble commented Jun 13, 2024

Description

Filter out unused IRFO Permit types from drop down

Related issue: VOL5009

Before submitting (or marking as "ready for review")

  • Does the pull request title follow the conventional commit specification?
  • Have you performed a self-review of the code
  • Have you have added tests that prove the fix or feature is effective and working
  • Did you make sure to update any documentation relating to this change?

Comment on lines +82 to +92
/**
* Filter unused IDs out of the array for display (cant remove from DB as 100s of permit records still reference the ID, but not needed for new permits)
*
* @param array $data Data
*
* @return array
*/
public function filterArrayById(array $data): array
{
return array_filter($data, fn($record) => !in_array($record['id'], self::UNUSED_TYPE_IDS));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Would this functionality be more appropriate as a database column we filter on? A soft delete of sorts that controls visibility?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was one approach i considered. Making schema changes, probably a new Repository method, changes to the query handler to call that method etc seemed like more to un-pick when the dependent permit records are finally deleted and this isnt needed anymore. Happy to re-factor in that pattern if you think its worthwhile though.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd say let's refactor to make it easier for us in the future. Decoupling the application from the data is also a positive.

As these agreements usually have a date associated with them, I'd recommend instead of a boolean of "hidden", let's throw a (nullable) date in there instead. Then in the code, if the row has a date populated do a comparison with today. This will enable us to hide these rows on a particular date (expiration of an agreement - which can be done on creation - then we'd not need this type of ticket at all).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No problem ill make the updates. 👍

@fibble fibble closed this Jun 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants