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

Feature: implement new validators #139

Merged

Conversation

syphax-bouazzouni
Copy link

@syphax-bouazzouni syphax-bouazzouni commented Feb 28, 2023

Prerequisites

Context

See agroportal/project-management#393

Changes

  • Add instance proc validators (8b14505)

Now we can call, model-specific validators.

For example, if a model defines an instance method called equal_to_10 that tests if a value is equal to 10.

Then we can reference it as a validator for an attribute, like so attribute :age, enforce: [:equal_to_10], this will enforce the attribute age to be 10 (as implemented in the method equal_to_10)

  • Add distinct_of validator (0be7925)
p1 distinct_of p2 enforce: A p1 B ^ A p2 C -> B <> C
  • Add superior_equla_to validator (deed329)
p1 superior_equla_to p2 enforce: A p1 B ^ A p2 C -> C <= B
  • Add email validator , that enforce the string to be a valid email

  • Add symmetric validator (5164723)

p is symmetric enforce: A p B -> B p A
  • Add inverse_of validator (d57a719)
p1 inverse_of p2 enforce: A p1 B  -> B p2 A

@syphax-bouazzouni syphax-bouazzouni changed the title Pr/feature/implement new validators Feature: implement new validators Feb 28, 2023
@mdorf mdorf self-assigned this Aug 29, 2023
@mdorf mdorf merged commit 88bb469 into ncbo:master Aug 29, 2023
@mdorf
Copy link
Member

mdorf commented Aug 30, 2023

We are getting this error when running ncbo/ontologies_linked_data unit tests:

	 2: from /Users/mdorf/dev/ncbo/goo/lib/goo/validators/validator.rb:94:in `empty_to_s?'
	 1: from /Users/mdorf/dev/ncbo/ontologies_linked_data/lib/ontologies_linked_data/models/users/user.rb:93:in `to_s'
/Users/mdorf/dev/ncbo/goo/lib/goo/base/settings/settings.rb:272:in `block in shape_attribute': Attribute `username` is not loaded for http://data.bioontology.org/users/tim. Loaded attributes: #<Set: {}>. (Goo::Base::AttributeNotLoaded)
rake aborted!

Is there another pull request inside ontologies_linked_data that we are missing?

@mdorf
Copy link
Member

mdorf commented Aug 30, 2023

With the new validators in place, it appears that the "username" of the User object is automatically assumed to have been loaded, which is not always the case, hence these errors. This errors out in multiple places in our test code.

@mdorf
Copy link
Member

mdorf commented Aug 30, 2023

These types of errors would be almost impossible to detect unit after the code is merged and deployed. We should be more diligent about submitting pull requests that are not fully self contained (or at least without having the additional dependencies clearly documented in the PR).

@mdorf
Copy link
Member

mdorf commented Aug 31, 2023

@syphax-bouazzouni, just wanted to check if you saw my original error report. The code errors out at:

      def to_s
        self.username.to_s
      end

Adding this code fixes many of these errors, but not all:

      def to_s
        self.bring(:username) if self.bring?(:username)
        self.username.to_s
      end

Do you have a different fix to this? This issue prevents us from proceeding.

@syphax-bouazzouni
Copy link
Author

syphax-bouazzouni commented Aug 31, 2023

Hi @mdorf ,

I just did this PR, that fixed the failing test in ontologies-linked-data (https://github.com/ontoportal-lirmm/ontologies_linked_data/actions/runs/6042678133), sorry I fixed this locally but forgot to do a PR for NCBO.

A better solution (i think) would be also to update the function empty_to_s?(object), like so

   def empty_to_s?(object)
        begin 
           object && object.to_s&.strip.empty?
       rescue 
         return true
       end
  end

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