Code Monkey home page Code Monkey logo

Comments (21)

nyrf avatar nyrf commented on August 30, 2024 1

@leastbad Thanks, that works.

from optimism.

nyrf avatar nyrf commented on August 30, 2024

If it's a nested form, it'll run more times like this

 ↳ app/controllers/posts_controller.rb:59:in `block (2 levels) in update'
  TaskItem Load (0.1ms)  SELECT "task_items".* FROM "task_items" WHERE "task_items"."post_id" = ?  [["post_id", 1]]
  ↳ app/controllers/posts_controller.rb:59:in `block (2 levels) in update'
  TaskItem Exists? (0.1ms)  SELECT 1 AS one FROM "task_items" WHERE "task_items"."name" = ? AND "task_items"."id" != ? AND "task_items"."post_id" = ? LIMIT ?  [["name", "11112"], ["id", 1], ["post_id", 1], ["LIMIT", 1]]
  ↳ app/controllers/posts_controller.rb:59:in `block (2 levels) in update'
  CACHE TaskItem Exists? (0.0ms)  SELECT 1 AS one FROM "task_items" WHERE "task_items"."name" = ? AND "task_items"."id" != ? AND "task_items"."post_id" = ? LIMIT ?  [["name", "11112"], ["id", 1], ["post_id", 1], ["LIMIT", 1]]
  ↳ app/controllers/posts_controller.rb:59:in `block (2 levels) in update'
  CACHE TaskItem Exists? (0.0ms)  SELECT 1 AS one FROM "task_items" WHERE "task_items"."name" = ? AND "task_items"."id" != ? AND "task_items"."post_id" = ? LIMIT ?  [["name", "11112"], ["id", 1], ["post_id", 1], ["LIMIT", 1]]
  ↳ app/controllers/posts_controller.rb:59:in `block (2 levels) in update'
  CACHE TaskItem Exists? (0.0ms)  SELECT 1 AS one FROM "task_items" WHERE "task_items"."name" = ? AND "task_items"."id" != ? AND "task_items"."post_id" = ? LIMIT ?  [["name", "11112"], ["id", 1], ["post_id", 1], ["LIMIT", 1]]
  ↳ app/controllers/posts_controller.rb:59:in `block (2 levels) in update'
  TaskItem Exists? (0.1ms)  SELECT 1 AS one FROM "task_items" WHERE "task_items"."name" = ? AND "task_items"."id" != ? AND "task_items"."post_id" = ? LIMIT ?  [["name", "11112"], ["id", 5], ["post_id", 1], ["LIMIT", 1]]
  ↳ app/controllers/posts_controller.rb:59:in `block (2 levels) in update'
  CACHE TaskItem Exists? (0.0ms)  SELECT 1 AS one FROM "task_items" WHERE "task_items"."name" = ? AND "task_items"."id" != ? AND "task_items"."post_id" = ? LIMIT ?  [["name", "11112"], ["id", 5], ["post_id", 1], ["LIMIT", 1]]
  ↳ app/controllers/posts_controller.rb:59:in `block (2 levels) in update'
  CACHE TaskItem Exists? (0.0ms)  SELECT 1 AS one FROM "task_items" WHERE "task_items"."name" = ? AND "task_items"."id" != ? AND "task_items"."post_id" = ? LIMIT ?  [["name", "11112"], ["id", 5], ["post_id", 1], ["LIMIT", 1]]
  ↳ app/controllers/posts_controller.rb:59:in `block (2 levels) in update'
  CACHE TaskItem Exists? (0.0ms)  SELECT 1 AS one FROM "task_items" WHERE "task_items"."name" = ? AND "task_items"."id" != ? AND "task_items"."post_id" = ? LIMIT ?  [["name", "11112"], ["id", 5], ["post_id", 1], ["LIMIT", 1]]
  ↳ app/controllers/posts_controller.rb:59:in `block (2 levels) in update'
  TaskItem Exists? (0.1ms)  SELECT 1 AS one FROM "task_items" WHERE "task_items"."name" = ? AND "task_items"."id" != ? AND "task_items"."post_id" = ? LIMIT ?  [["name", "11122"], ["id", 8], ["post_id", 1], ["LIMIT", 1]]
  ↳ app/controllers/posts_controller.rb:59:in `block (2 levels) in update'
  CACHE TaskItem Exists? (0.0ms)  SELECT 1 AS one FROM "task_items" WHERE "task_items"."name" = ? AND "task_items"."id" != ? AND "task_items"."post_id" = ? LIMIT ?  [["name", "11122"], ["id", 8], ["post_id", 1], ["LIMIT", 1]]
  ↳ app/controllers/posts_controller.rb:59:in `block (2 levels) in update'
  CACHE TaskItem Exists? (0.0ms)  SELECT 1 AS one FROM "task_items" WHERE "task_items"."name" = ? AND "task_items"."id" != ? AND "task_items"."post_id" = ? LIMIT ?  [["name", "11122"], ["id", 8], ["post_id", 1], ["LIMIT", 1]]
  ↳ app/controllers/posts_controller.rb:59:in `block (2 levels) in update'
  CACHE TaskItem Exists? (0.0ms)  SELECT 1 AS one FROM "task_items" WHERE "task_items"."name" = ? AND "task_items"."id" != ? AND "task_items"."post_id" = ? LIMIT ?  [["name", "11122"], ["id", 8], ["post_id", 1], ["LIMIT", 1]]
  ↳ app/controllers/posts_controller.rb:59:in `block (2 levels) in update'
  CACHE Post Exists? (0.0ms)  SELECT 1 AS one FROM "posts" WHERE "posts"."name" = ? AND "posts"."id" != ? LIMIT ?  [["name", "222"], ["id", 1], ["LIMIT", 1]]

from optimism.

leastbad avatar leastbad commented on August 30, 2024

Could you please add some context? I am not in your brain. Please use your words.

from optimism.

leastbad avatar leastbad commented on August 30, 2024

Are you saying that I should use model.errors.any? instead of model.invalid?

You never actually explain yourself, and your comments are assumed to be self-evident. I sincerely appreciate feedback, but I need a lot more information from you to put this to work.

Are you working with the currently published gem or this master branch? They are quite different, which is part of the reason I'm frustrated by your lack of context.

from optimism.

nyrf avatar nyrf commented on August 30, 2024

Sorry for my weak english, I use the master branch and base on the demo code https://github.com/leastbad/optimism-demo, i found when the post model invalid because of unique name, the log likes:

Post Exists? (0.2ms)  SELECT 1 AS one FROM "posts" WHERE "posts"."name" = ? AND "posts"."id" != ? LIMIT ?  [["name", "222"], ["id", 1], ["LIMIT", 1]]
  ↳ app/controllers/posts_controller.rb:59:in `block (2 levels) in update'
  CACHE Post Exists? (0.0ms)  SELECT 1 AS one FROM "posts" WHERE "posts"."name" = ? AND "posts"."id" != ? LIMIT ?  [["name", "222"], ["id", 1], ["LIMIT", 1]]
  ↳ app/controllers/posts_controller.rb:59:in `block (2 levels) in update'
  CACHE Post Exists? (0.0ms)  SELECT 1 AS one FROM "posts" WHERE "posts"."name" = ? AND "posts"."id" != ? LIMIT ?  [["name", "222"], ["id", 1], ["LIMIT", 1]]
  ↳ app/controllers/posts_controller.rb:59:in `block (2 levels) in update'
  CACHE Post Exists? (0.0ms)  SELECT 1 AS one FROM "posts" WHERE "posts"."name" = ? AND "posts"."id" != ? LIMIT ?  [["name", "222"], ["id", 1], ["LIMIT", 1]]
  ↳ app/controllers/posts_controller.rb:59:in `block (2 levels) in update'
  CACHE Post Exists? (0.0ms)  SELECT 1 AS one FROM "posts" WHERE "posts"."name" = ? AND "posts"."id" != ? LIMIT ?  [["name", "222"], ["id", 1], ["LIMIT", 1]]
  ↳ app/controllers/posts_controller.rb:59:in `block (2 levels) in update'
[ActionCable] Broadcasting to OptimismChannel: {"cableReady"=>true, "operations"=>{"addCssClass"=>[{"selector"=>"#post_age_container", "name"=>"error"}, {"selector"=>"#post_form", "name"=>"invalid"}], "removeCssClass"=>[{"selector"=>"#post_name_container", "name"=>"error"}, {"selector"=>"#post_body_container", "name"=>"error"}, {"selector"=>"#post_consent_container", "name"=>"error"}], "textContent"=>[{"selector"=>"#post_name_error", "text"=>""}, {"selector"=>"#post_age_error", "text"=>"Age must be greater than or equal to 18"}, {"selector"=>"#post_body_error", "text"=>""}, {"selector"=>"#post_consent_error", "text"=>""}]}}

after i modify the code https://github.com/leastbad/optimism/blob/master/lib/optimism.rb#L73

if model.invalid? && model.errors.messages.map(&:first).include?(attribute.to_sym) to if model.errors.any? && model.errors.messages.map(&:first).include?(attribute.to_sym)

 Post Exists? (0.2ms)  SELECT 1 AS one FROM "posts" WHERE "posts"."name" = ? AND "posts"."id" != ? LIMIT ?  [["name", "222"], ["id", 1], ["LIMIT", 1]]
  ↳ app/controllers/posts_controller.rb:59:in `block (2 levels) in update'
[ActionCable] Broadcasting to OptimismChannel: {"cableReady"=>true, "operations"=>{"addCssClass"=>[{"selector"=>"#post_age_container", "name"=>"error"}, {"selector"=>"#post_form", "name"=>"invalid"}], "removeCssClass"=>[{"selector"=>"#post_name_container", "name"=>"error"}, {"selector"=>"#post_body_container", "name"=>"error"}, {"selector"=>"#post_consent_container", "name"=>"error"}], "textContent"=>[{"selector"=>"#post_name_error", "text"=>""}, {"selector"=>"#post_age_error", "text"=>"Age must be greater than or equal to 18"}, {"selector"=>"#post_body_error", "text"=>""}, {"selector"=>"#post_consent_error", "text"=>""}]}}

from optimism.

nyrf avatar nyrf commented on August 30, 2024

Especially with nested form, eg:

class Post < ApplicationRecord
  validates :name, :age, :body, presence: true
  validates :name, uniqueness: true
  validates :age, numericality: { only_integer: true, greater_than_or_equal_to: 18 }
  validates :body, length: { minimum: 10 }
  validates :consent, acceptance: { message: "must be given" }


  has_many :task_items 

  accepts_nested_attributes_for :task_items, allow_destroy: true
end
class TaskItem < ApplicationRecord
  validates :name, presence: true, uniqueness: true
  belongs_to :post
end

when save post fail because of duplicate task_item name, the log likes #8 (comment), use if model.errors.any? should solve this problem.

from optimism.

leastbad avatar leastbad commented on August 30, 2024

Thanks for following up, @nyrf. I didn't mean to imply that there was anything wrong with your English. I actually needed more of it! :)

I'm working to figure out what's going on right now. If you look in the gem version, you'll actually see that I used to have model.errors.any? and then I received a very good PR from @joshleblanc that gave a compelling reason to change it. I am now working hard to verify how it was vs. how it works now. Keeping it all in my head is honestly a bit stressful but I really want to get this right for everyone. You can see the PR in question at #7.

from optimism.

joshleblanc avatar joshleblanc commented on August 30, 2024

invalid? (and valid?) perform the server-side validations on the model. (And therefor populate errors).

Without the invalid? call, the model is never validated, and the errors object will be empty (unless some method is called that performs validation, like save).

Is the problem that it's running the validations multiple times? That would be possible. We could just document that validations aren't run by optimism, and you have to do that manually.

All of those additional logs look like cache hits though, so it doesn't actually look like a performance problem.

Sorry, I'm not 100% clear on what the actual issue is 😄

from optimism.

leastbad avatar leastbad commented on August 30, 2024

@nyrf still working on this. Could you share your relevant models? Or at least tell me if you are validating uniqueness?

validates :name, uniqueness: true

All of those Post exists? queries are uniqueness validations.

What I can confirm right now, at least in the optimism-demo app, is that something really weird is going on.

@joshleblanc the actual issue is that the library is behaving differently in subtle ways. It's cool that it's working for you, and I genuinely love the tweaks you made, but... you aren't the only user.

from optimism.

leastbad avatar leastbad commented on August 30, 2024

Okay @joshleblanc @nyrf I think I have a reasonable, working compromise that will ensure validation and cut down on CACHE log spam.

9711063

What @joshleblanc helped me understand is that if validations are never called, the errors collection will always be empty. However, there's no need to validate the entire model over and over again, either.

So what I did was add a line: model.valid? if model.errors.empty? before the processing begins. This should properly populate the collection, allowing me to switch back to model.errors.any? This will potentially add one additional validation pass, but I think this is an acceptable tradeoff because I don't want to have to tell people to call valid on the model just to use this library.

I did keep the guarding check on line 73/74, but changed it to model.errors.any? as well. @joshleblanc I am working on the assumption here that the nested models have already had valid? called on them when the parent model is validated... so the error collection should already be built.

I also changed the Optimism.channel lambda to return "OptimismChannel" as a default. I will be giving detail instructions for authenticated configuration in the docs.

Does this work for you guys?

from optimism.

nyrf avatar nyrf commented on August 30, 2024

@leastbad I found when i use https://github.com/toptal/database_validations to validate nested model uniqueness, seems can't show nested model unique error message. this is the demo code : https://github.com/nyrf/optimism-demo/tree/nested

Kapture 2020-05-20 at 10 30 56

WX20200520-102647@2x

from optimism.

leastbad avatar leastbad commented on August 30, 2024

@nyrf thanks for spotting this. I was just writing you a note to explain that the optimism-demo repo needs to be updated to work with the latest gem which hasn't been published yet. However, it seems like you're seeing an entirely different problem. I'm looking into it.

I am unfamiliar with the database_validations gem. Can you verify that you're still having this problem when not using the gem?

from optimism.

nyrf avatar nyrf commented on August 30, 2024

@leastbad I found that because of the database_validations gem seems not trigger nested model invalid when use db_uniqueness validates :body, presence: true, db_uniqueness: { scope: :post_id }, this is my debug code:

WX20200520-112722@2x

You can see that nested model comment has errors, but it invalid? is false. i'll try to modify the database_validations gem, thanks.

from optimism.

leastbad avatar leastbad commented on August 30, 2024

Just so I understand, are you saying that when you removed the database_validations gem from the test scenario, nested model validation reporting behaved correctly? I don't want to ship this if you are seeing weird problems.

from optimism.

nyrf avatar nyrf commented on August 30, 2024

I use database_validations to validate comment model body column uniqueness when create or update at once. eg:

post = Post.new({name: 'name', age: 19, body: 'bodybodybody'})
comment = Comment.new(body: 'body')
comment2 = Comment.new(body: 'body')
post.comments = [comment, comment2]
post.save
19] pry(main)> post.errors
=> #<ActiveModel::Errors:0x00007fe0ca6045e8
 @base=
  #<Post:0x00007fe0ccb73590
   id: nil,
   name: "name",
   age: 19,
   body: "bodybodybody",
   consent: nil,
   created_at: Wed, 20 May 2020 04:27:18 UTC +00:00,
   updated_at: Wed, 20 May 2020 04:27:18 UTC +00:00>,
 @details={},
 @messages={}>
pry(main)> post.comments.last.errors
=> #<ActiveModel::Errors:0x00007fe0cbc714e8
 @base=
  #<Comment:0x00007fe0ca5483c0
   id: nil,
   post_id: 30,
   body: "body",
   created_at: Wed, 20 May 2020 04:27:18 UTC +00:00,
   updated_at: Wed, 20 May 2020 04:27:18 UTC +00:00>,
 @details={:body=>[{:error=>:taken, :value=>"body"}]},
 @messages={:body=>["has already been taken"]}>
pry(main)> post.invalid?
  Post Exists? (0.3ms)  SELECT 1 AS one FROM "posts" WHERE "posts"."name" = $1 LIMIT $2  [["name", "name"], ["LIMIT", 1]]
  Comment Exists? (0.3ms)  SELECT 1 AS one FROM "comments" WHERE "comments"."body" = $1 AND "comments"."post_id" = $2 LIMIT $3  [["body", "body"], ["post_id", 30], ["LIMIT", 1]]
  Comment Exists? (0.3ms)  SELECT 1 AS one FROM "comments" WHERE "comments"."body" = $1 AND "comments"."post_id" = $2 LIMIT $3  [["body", "body"], ["post_id", 30], ["LIMIT", 1]]
=> false
[24] pry(main)> post.comments.last.invalid?
  Comment Exists? (0.4ms)  SELECT 1 AS one FROM "comments" WHERE "comments"."body" = $1 AND "comments"."post_id" = $2 LIMIT $3  [["body", "body"], ["post_id", 30], ["LIMIT", 1]]
=> false

If not use the database_validations gem. can't valid nested model uniqueness at once, see rails/rails#20676 . other validates works fine.

from optimism.

leastbad avatar leastbad commented on August 30, 2024

Okay, thank you. Unfortunately, I have to slept but I'll be back on this tomorrow. Thanks to both of you.

from optimism.

nyrf avatar nyrf commented on August 30, 2024

Thanks for your great work, I found an ugly way to make accepts_nested_attributes_for method can works with uniqueness and optimism.

class MyValidator < ActiveModel::Validator
  def validate(record)
    invalid = false
    names = record.comments.map(&:body)
    record.comments.each_with_index do |item, index|
      if names.count(item.body) > 1
        item.errors.add(:body, :taken, value: item.body, message: "unique invalid")
        invalid = true
      end
    end
    record.errors.add("comments.body", "invalid") if invalid
  end
end

class Post < ApplicationRecord
  validates :name, :age, :body, presence: true
  validates :name, uniqueness: true
  validates :age, numericality: {only_integer: true, greater_than_or_equal_to: 18}
  validates :body, length: {minimum: 10}
  validates :consent, acceptance: {message: "must be given"}

  has_many :comments, dependent: :destroy
  accepts_nested_attributes_for :comments, allow_destroy: true
  validates_with MyValidator
end

WX20200520-142304@2x

from optimism.

leastbad avatar leastbad commented on August 30, 2024

Hello @nyrf, I hope that you missed me.

I have just pushed a significant update of the optimism-demo project to master on GH.

What I recommend is that you clone/pull optimism and run bundle config set local.optimism ~/optimism (adjusting for where you have the code). https://bundler.io/v2.0/guides/git.html#local

Then pull the latest changes for optimism-demo and update the Gemfile so that it reads: gem "optimism", github: "leastbad/optimism", branch: "master" while you are testing.

There is a migration to run but otherwise it should be ready to run if you already had it working.

I believe that it demonstrates everything working as expected, without any ugly hacks. It could be that you can pull some techniques from the models and views in the project. Let me know if you have any suggestions - I don't want it to be too complex, though.

Please give this a try while I work on documentation for the new release. If everything looks good to you, I will close this issue and you can expect a formal release soon, probably tonight.

from optimism.

nyrf avatar nyrf commented on August 30, 2024

@leastbad Yes, it works, but if i want to validate uniqueness of comment model's opinion column, i've to use the ugly code, that is a bug of rails not optimism, you can close the issue, thanks for your help.

from optimism.

leastbad avatar leastbad commented on August 30, 2024

I understand. I did a quick Google search and saw this: https://stackoverflow.com/questions/24416599/rails-validate-nested-attributes-uniqueness-with-scope-parent-of-parent

Thanks for sticking with this and let me know if you notice anything else.

from optimism.

nyrf avatar nyrf commented on August 30, 2024

Yes, i've, this is a bug of rails for many years, the only way is to write custom ugly codes to do that now. i used #8 (comment) and it works.

from optimism.

Related Issues (12)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.