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

resource.record nil in a field block (association) #2544

Closed
4 of 11 tasks
jetienne opened this issue Feb 29, 2024 · 16 comments · Fixed by #2547
Closed
4 of 11 tasks

resource.record nil in a field block (association) #2544

jetienne opened this issue Feb 29, 2024 · 16 comments · Fixed by #2547

Comments

@jetienne
Copy link

jetienne commented Feb 29, 2024

Describe the bug

Upgrade from 3.4.1 to 3.4.2 broke the following code, the resource record is always nil.

  field :state, as: :badge, options: {
        info: 'sent',
        success: 'accepted',
        danger: 'refused'
      }, name: '', visible: lambda {
        resource.record.state.in?(%w[accepted refused])
      }

The exception is undefined method state' for nil`.

By just replacing 3.4.1 to 3.4.2 in the same environment (just reloading the page) it fails.

Our models are Deal has_many Proposaland it fails on a deals#show with has_many :proposals having the field state.

We had to revert our production because of this bug.

System configuration

Avo version:
3.4.2

Rails version:
7.1.3.2

Ruby version:
3.3.0

License type:

  • Community
  • Pro
  • Advanced

Are you using Avo monkey patches, overriding views or view components?

  • Yes. If so, please post code samples.
  • No

Impact

  • High impact (It makes my app un-usable.)
  • Medium impact (I'm annoyed, but I'll live.)
  • Low impact (It's really a tiny thing that I could live with.)

Urgency

  • High urgency (I can't continue development without it.)
  • Medium urgency (I found a workaround, but I'd love to have it fixed.)
  • Low urgency (It can wait. I just wanted you to know about it.)

(we cannot upgrade until it's fixed).

@Paul-Bob
Copy link
Contributor

Hello @jetienne is that happening on index?

@jetienne
Copy link
Author

@Paul-Bob I just updated the issue
Our models are Deal has_many Proposaland it fails on a deals#show with has_many :proposals having the field state.

@Paul-Bob
Copy link
Contributor

Can you please confirm if record instead resource.record have the same output?

@Paul-Bob
Copy link
Contributor

Paul-Bob commented Feb 29, 2024

We fixed a bug where resource.record would always be the last record from the index page.

I think if you do

 field :state, as: :badge, options: {
        info: 'sent',
        success: 'accepted',
        danger: 'refused'
      }, name: '', visible: lambda {
        puts ["resource.record->", resource.record].inspect
        resource.record.state.in?(%w[accepted refused])
      }

resource.record-> will output always the same record, the last of the table

@jetienne
Copy link
Author

jetienne commented Feb 29, 2024

@Paul-Bob record fails also, being nil

@Paul-Bob
Copy link
Contributor

@jetienne can you confirm the puts ["resource.record->", resource.record].inspect on avo 3.4.1 I think it will output always the last record of the table

@jetienne
Copy link
Author

jetienne commented Feb 29, 2024

@Paul-Bob If i have 2 proposals in my deal, this is the log

web    | ["LOGPAUL resource.record->", #<Proposal id: "fda24de9-37dc-4aec-b715-c165ed5f58ee", created_at: "2024-02-29 16:12:29.638171000 +0000", updated_at: "2024-02-29 16:12:29.638171000 +0000", state: "sent", option_deadline_on: nil, booking_amount: 0.15e3, check_in_on: "2024-02-29", check_out_on: "2024-03-01", villa_id: "a3de68ad-feb7-4b79-a60d-88e5bdefc006", deal_id: "4dede0f0-f773-4090-9659-a3f79d39aaea">]
web    |   Rendered /Users/jetienne/.rbenv/versions/3.3.0/lib/ruby/gems/3.3.0/gems/avo-3.4.1/app/views/avo/partials/_table_header.html.erb (Duration: 1.2ms | Allocations: 810)
web    | ["LOGPAUL resource.record->", #<Proposal id: "2aa73792-e182-42b7-a4c8-4c2775a9e7c9", created_at: "2024-02-29 16:12:42.018409000 +0000", updated_at: "2024-02-29 16:12:42.018409000 +0000", state: "sent", option_deadline_on: nil, booking_amount: 0.1565e4, check_in_on: "2024-03-01", check_out_on: "2024-03-23", villa_id: "a3de68ad-feb7-4b79-a60d-88e5bdefc006", deal_id: "4dede0f0-f773-4090-9659-a3f79d39aaea">]
web    | Started GET "/avo-dashboards-assets/avo_dashboards.js.map" for 127.0.0.1 at 2024-02-29 17:13:48 +0100
web    | ["LOGPAUL resource.record->", #<Proposal id: "fda24de9-37dc-4aec-b715-c165ed5f58ee", created_at: "2024-02-29 16:12:29.638171000 +0000", updated_at: "2024-02-29 16:12:29.638171000 +0000", state: "sent", option_deadline_on: nil, booking_amount: 0.15e3, check_in_on: "2024-02-29", check_out_on: "2024-03-01", villa_id: "a3de68ad-feb7-4b79-a60d-88e5bdefc006", deal_id: "4dede0f0-f773-4090-9659-a3f79d39aaea">]
$```

No idea why it's being called 3 times instead of 2, but as you can see all records are being logged.

@Paul-Bob
Copy link
Contributor

And on 3.4.2 with this code?

 field :state, as: :badge, options: {
        info: 'sent',
        success: 'accepted',
        danger: 'refused'
      }, name: '', visible: lambda {
        puts ["resource.record->", resource&.record].inspect
        # resource.record.state.in?(%w[accepted refused])
        true
      }

I'm trying to replicate it on my side but if you can check this one on same conditions would be helpful

@jetienne
Copy link
Author

jetienne commented Feb 29, 2024

web    | LOGLOG
web    | ["resource.record->", nil]
web    |   Rendered /Users/jetienne/.rbenv/versions/3.3.0/lib/ruby/gems/3.3.0/gems/avo-3.4.2/app/views/avo/partials/_table_header.html.erb (Duration: 0.4ms | Allocations: 330)
web    | LOGLOG
web    | ["resource.record->", #<Proposal id: "2aa73792-e182-42b7-a4c8-4c2775a9e7c9", created_at: "2024-02-29 16:12:42.018409000 +0000", updated_at: "2024-02-29 16:12:42.018409000 +0000", state: "sent", option_deadline_on: nil, booking_amount: 0.1565e4, check_in_on: "2024-03-01", check_out_on: "2024-03-23", villa_id: "a3de68ad-feb7-4b79-a60d-88e5bdefc006", deal_id: "4dede0f0-f773-4090-9659-a3f79d39aaea">]
web    | LOGLOG
web    | ["resource.record->", #<Proposal id: "fda24de9-37dc-4aec-b715-c165ed5f58ee", created_at: "2024-02-29 16:12:29.638171000 +0000", updated_at: "2024-02-29 16:12:29.638171000 +0000", state: "sent", option_deadline_on: nil, booking_amount: 0.15e3, check_in_on: "2024-02-29", check_out_on: "2024-03-01", villa_id: "a3de68ad-feb7-4b79-a60d-88e5bdefc006", deal_id: "4dede0f0-f773-4090-9659-a3f79d39aaea">]

@jetienne
Copy link
Author

jetienne commented Feb 29, 2024

@Paul-Bob It seems that for 2 items, it's being called 3 items, the first one on a nil proposal (in my case)

@Paul-Bob
Copy link
Contributor

Yes, first call is to render header. On header Avo fetches fields from the resource in order to know the columns to print. Then the block gets evaluated again for each row (record in particular).

Notice that 2nd and 3rd logs are the calls for each row.

The first one (header) since is a index render, the resource has no record. Previous, on versions lower than 4.3.2 we had a bug where the resource was hydrated with the last record on the table that's why on the logs from version 4.3.1 the log from header is printing the id of the last record on the table.

Hope it makes sense, let me know if something still unclear

@jetienne
Copy link
Author

Why would you call it for the header? The header is not a resource, it's purely decorative.

@jetienne
Copy link
Author

jetienne commented Feb 29, 2024

Moreover, it means that from 3.4.2, we need to review ALL the field visible blocks like

field :pending_email, as: :text, protocol: :mailto,
                  name: 'Pending Email Verification', visible: -> { resource.record.user.present? && resource.record.user.unconfirmed_email.present? } do
              record.user.unconfirmed_email if record.user.present?
            end

when it was smoothly managed before.

@Paul-Bob
Copy link
Contributor

Why would you call it for the header? The header is not a resource, it's purely decorative.

If fields visibility is not checked for header a filed like this would be rendered anyway on the header

field :name, as: :text, visible: -> { !view.index? }

The ability to hide columns on index is necessary

Moreover, it means that from 3.4.2, we need to review ALL the field visible blocks like

This is consequence of a bug fix, not ideal I know

when it was smoothly managed before.

Was not breaking but if the visible block of the last record of t he table was returning false that column for all records on the table was being hidden on index.

@jetienne
Copy link
Author

Thanks for the clarifications, can we be noticed on the release notes when there are such breaking changes?

@Paul-Bob
Copy link
Contributor

Yes @jetienne of course. We add all the known breaking changes to update section, just updated with this one, didn't realize it before. Thank you for reporting it!

@Paul-Bob Paul-Bob mentioned this issue Mar 1, 2024
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants