#105 Allow Procs for dynamic value usage

[codegeek319]

This PR allows Procs for the value determinations at runtime. Also the Test and the Documentation is updated according to the latest code. closes #105

[gr8bit]

@igorkasyanchuk would you mind to take another look? Thanks! :)

[igorkasyanchuk]

Hello @gr8bit ,

Sorry for taking so long time, I think I've some important comments now. I see in the code that often you use something like range = range.call(record) if range.is_a?(Proc) and you checking in many places objects if it's proc or now. Because of it you changed lot's of code, and added some complexity to existing method, for example by adding new parameters.

I've the following idea, it probably can take not long time to implement it, at least I want you to think if it could work better. So the idea is the following - validation class receives some "options", which are consist of hash and potentially hashes inside.

We can just convert all hashes to values before. So no need to call proc somewhere inside validator. Everything will keep working as before.

I did some quick prototype, you can use it as a reference and improve. Ready to discuss it with you, If needed in skуpe "igorkasyanchuk".

[igorkasyanchuk]

@codegeek319 sorry mentioning you too. Just in case you need:

    def recur_call_hash(record, h)
      hash = h.dup
      return hash unless hash.is_a?(Hash)

      puts hash
      hash.each do |(k,v)|
        if v.is_a?(Proc)
          hash[k] = v.call(record)
        else
          hash[k] = v
        end
        if hash[k].is_a?(Hash)
          hash[k] = recur_call_hash(record, hash[k])
        end
      end
      hash
    end

[igorkasyanchuk]

and last suggestion, to make in Readme example more close to real life if possible. For example

validates :proc_files, limit: { max: -> (record) { record.admin? ? 100 : 10 } }

[gr8bit]

Thank you for the great feedback!! @codegeek319 has been implementing this and we'll discuss it on Tuesday, we work together irl. :)

[igorkasyanchuk]

@gr8bit @codegeek319 do you have plans to fix this PR?

[gr8bit]

Sure thing! Sorry, we have been quite busy ☺️

[gr8bit]

@igorkasyanchuk finally I found the time for the refactoring you suggested. Diff shrunk significantly. Would you mind taking another look? :)

[igorkasyanchuk]

thank you for your PR, from my side I think it looks good

But I want to check with other developers who did the most contributions to the gem

@StefSchenkelaars , @phlegx , @tleneveu , @reckerswartz what do you think about this PR?

[gr8bit]

@StefSchenkelaars I commented on and coded towards your suggestions. :) What do you think?

[StefSchenkelaars]

I still don't really like the passing of these options all the time but I'm ok if it works.

[gr8bit]

I still don't really like the passing of these options all the time but I'm ok if it works.

I'm totally open to suggestions! Please check the validators and find a better/easier way - keep in mind though the options have to be processed on every validation.

[gr8bit]

I think this PR could be merged now.

[StefSchenkelaars]

@igorkasyanchuk Agree with @gr8bit that it's good for a merge. I think you are the only one who can do that.

[igorkasyanchuk]

Great) will do it later today Thanks all for tremendous amount of work

[igorkasyanchuk]

active_storage_validations-1.0.0.gem released

[igorkasyanchuk]

looks like one regression https://github.com/igorkasyanchuk/active_storage_validations/issues/166 if someone can take a look on this