3351 add family archive

[grantmca]

Checklist:

• I have performed a self-review of my own code,
• I have added tests that prove my fix is effective or that my feature works,
• New and existing unit tests pass locally with my changes ("bundle exec rake"),
• Title include "WIP" if work is in progress.

Resolves #3351

Description

Guide questions:

• What motivated this change (if not already described in an issue)?
  --The ability to archive families no longer use to help users not have to scroll through large list
  --Increases usability for the user, provides additional info on reports

Type of change

• New feature (non-breaking change which adds functionality)

How Has This Been Tested?

You can validate the change by switching some families to archived in the dev env and ensuring they do not show up by default, additionally check that any children associated with the families are also archived. Also validate that when "Include Archived" is checked that all families are shown on the index screen

Screenshots

• To make things look good I centered both the checkbox on families and on children filters so they match

[dorner]

Thanks for the PR! Added some comments and suggestions.

[grantmca]

I think I have addressed all of the changes, I might need to rebase but should be set

[dorner]

Getting closer! Had some more comments.

[grantmca]

Getting closer! Had some more comments.

Thank you! I'll get on it

[grantmca]

I moved forward with all of the changes to the service class and the migration. I played around with updating the family controller for a while to make it work with bools but I did some investigation into filterrific and it when you pass in 'false' as a query parameter to filteriffic it translates that false to '0' and when you pass in true it is passed through as true. 0 is truthy in ruby and the True object does not have the method zero? on it so there is not good concise way to write the scope. I think the way it is now it the best option and the it follows the pattern of the children model as well.

[dorner]

@grantmca I'm not sure I understand this. Filterrific parameters get sent to the controller. The controller can make whatever changes it has to do to those parameters rather than passing them blindly to the service object or the model. I'd rather see those changes happen in the controller than downstream.

[grantmca]

@grantmca I'm not sure I understand this. Filterrific parameters get sent to the controller. The controller can make whatever changes it has to do to those parameters rather than passing them blindly to the service object or the model. I'd rather see those changes happen in the controller than downstream.

I apologize for any confusion caused by my previous explanation. Here's a more detailed breakdown: the view sends either '0' or '1' as the params for the checkbox to the controller. Within the controller, I implemented and tested logic that cast this '0' or '1' to a boolean value and then pass this boolean to the 'filterrific' initializer. The gem 'filterrific' provides us with the service object @filterrific, and its method @filterrific.find receives the filterrific parameters from the controller. Using these parameters and a scope defined on the model, the method returns the desired results. However, there's is a catch: the "filterrific" service object performs some parameter data cleanup before passing the parameters to the model's scope. One such cleanup action is converting 'false' to 0 before passing it to the model's scope. Since the model's scope might expect a boolean value, this can lead to converting the scope just for it to be converted back to 0. I am open to any suggestions on how to best handle this case! Let me know if you need any further adjustments!

[dorner]

I think the issue might be because it's using the string "false" instead of a Boolean false. Can you try casting the string to a boolean and see what happens?

[grantmca]

I think the issue might be because it's using the string "false" instead of a Boolean false. Can you try casting the string to a boolean and see what happens?

I am sorry I should of used backticks instead of single quotes, I am passing the bool false into the params when I was getting that response. I am struggling to figure out why this is happening. I have unpacked the gem and I am digging into it to try and see if I can come up with any reason, I will keep you updated with what I find

[dorner]

Please do, thanks. This behavior really doesn't make any sense.

[grantmca]

Please do, thanks. This behavior really doesn't make any sense.

This seems to maybe be a bug on Filterfific's end. On this line you can see that in the define_and_assign_attr_accessors_for_each_filter goes through each of the params for each filter and before assigning them as attr_accessor it checks each of them to see if they are present? and if not it will not assign them. In our case false.present? is false because false is treated as a non-present value so the parameter is not being passed into the scope. This does not seem like the intended behavior for the service obj so I am going to reach out on the filterrific gem and maybe create a PR to fix this issue. Given this info what would you recommend on this PR? @dorner

[dorner]

So it looks like this is the suggested approach: jhund/filterrific#9

In general I'd try to keep the 0/1 check to a single place. If you have to massage the parameters to turn the 0/1 into true/false, I'd do it in a method or callback before any other methods are called.

[grantmca]

So it looks like this is the suggested approach: jhund/filterrific#9

In general I'd try to keep the 0/1 check to a single place. If you have to massage the parameters to turn the 0/1 into true/false, I'd do it in a method or callback before any other methods are called.

I went ahead and opened a PR for this on filterrific jhund/filterrific#224 @dorner let me know how we should proceed form here.

[dorner]

@grantmca did you try this approach? jhund/filterrific#9 (comment) We could use that while we wait :)

[grantmca]

I think that we are all set now! @dorner, I updated the scope to follow the example on the issue you linked. Let me know what you think!

[dorner]

Probably as good as it's going to get - but I think there's a typo in the migration file?

[dorner]

Thanks for all your patience with this! In it goes!