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

Reorder key-value fields #1029

Open
iainbeeston opened this issue Jul 8, 2022 · 13 comments · May be fixed by #3299
Open

Reorder key-value fields #1029

iainbeeston opened this issue Jul 8, 2022 · 13 comments · May be fixed by #3299
Assignees
Labels
Enhancement New feature or request Good first issue Good for newcomers hacktoberfest Help wanted We could use some help with this

Comments

@iainbeeston
Copy link
Contributor

Feature

It would be helpful to reorder key-value fields. At the moment when you enter key values the only way to reorder them is to delete and re-add, which is difficult if you have many entries.

Current workarounds

Delete and re-add those entries. Alternatively, when you display them to the user (outside of avo) you could apply a sort, but only very simple sorting would be possible (like alphabetical ordering).

Screenshots

Screenshot 2022-07-08 at 17 10 14

Additional context

You can see this in the avo demo app

@adrianthedev
Copy link
Collaborator

That sounds like a good idea. It shouldn't be that difficult to implement. Maybe someone can pick it up.

  • We have icon assets for records re-ordering (arrow-up.svg & arrow-down.svg).
  • The JS logic should be to move an item in an array. The field holds the items as array of arrays in memory ([['hoho', 'hohoho'], ['Teste', 'abc'], ...] in the above example).
  • The JS logic should enable/disable the up/down buttons when the item is first/last
  • That's it 🤷‍♂️

@adrianthedev adrianthedev added Enhancement New feature or request Help wanted We could use some help with this Good first issue Good for newcomers labels Jul 10, 2022
@enderahmetyurt
Copy link

Hey @adrianthedev I can get it if it's still available 🙏

@adrianthedev
Copy link
Collaborator

Yes, you can!

@adrianthedev
Copy link
Collaborator

And, thank you!

@enderahmetyurt
Copy link

@adrianthedev you mean something like this 🤭

Screen.Recording.2024-10-01.at.18.02.24.mov

@enderahmetyurt
Copy link

Screen.Recording.2024-10-01.at.20.20.06.mov

even it works in front-end well, it cannot be updated on DB.

I checked what I sent to controller "meta"=>"{\"ahmet\":\"\",\"ender\":\"\",\"yurt\":\"\"}"} it looks like that but it looks on DB

3.3.1 :003 > p.meta_for_database
 => "{\"ender\":\"\",\"ahmet\":\"\",\"yurt\":\"\"}"
3.3.1 :004 > p.meta
 => {"ender"=>"", "ahmet"=>"", "yurt"=>""}

I am pretty new in AVO codebase. Where or what should I check?

@Paul-Bob
Copy link
Contributor

Paul-Bob commented Oct 1, 2024

Hey @enderahmetyurt, looking great already, thank you!

I would start by debugging it here https://github.com/avo-hq/avo/blob/main/lib/avo/fields/key_value_field.rb#L86.

The fill_field method is called to ensure that the data from the params is correctly parsed for each field before assigning it to the record and hitting the DB. There is a base fill_field and some fields override it, KeyValueField is one of those

@enderahmetyurt
Copy link

Hey @enderahmetyurt, looking great already, thank you!

I would start by debugging it here https://github.com/avo-hq/avo/blob/main/lib/avo/fields/key_value_field.rb#L86.

The fill_field method is called to ensure that the data from the params is correctly parsed for each field before assigning it to the record and hitting the DB. There is a base fill_field and some fields override it, KeyValueField is one of those

Thank you man. I'll look at it 🙏

@Paul-Bob
Copy link
Contributor

Paul-Bob commented Oct 1, 2024

Let me know how it goes

@enderahmetyurt
Copy link

I debugged it and I noticed that meta field cannot be updated since The keys in meta to be changed are the same as the previous one.

Screenshot 2024-10-02 at 09 15 06

but if I changed the fill_field method like this

  begin
    new_value = JSON.parse(value)
  rescue StandardError
    new_value = {}
  end
  
  record.update(meta: {}) if key == 'meta'
  
  record.send(:"#{key}=", new_value)
  
  record
Screen.Recording.2024-10-02.at.09.28.42.mov

it looks good but I don't like the solution. I'd like to hear your ideas 🙏

@Paul-Bob
Copy link
Contributor

Paul-Bob commented Oct 2, 2024

We can't do a direct update on fill_field method because it would be an early update without validations then we would have 2 updates on the same record per save request.

I suspected that this could be the issue, thanks for debugging it!

I'll do some research, meanwhile, let me know if you find anything.

While writing this, an idea just came to mind. What if we mark the key as dirty on the fill_field method, something like: record.send(:"#{key}_will_change!")

@enderahmetyurt
Copy link

  def fill_field(record, key, value, _params)
    begin
      new_value = JSON.parse(value)
    rescue StandardError
      new_value = {}
    end

    record.send(:"#{key}_will_change!")
    record.send(:"#{key}=", new_value)

    record
  end

it works fine like above :)

@Paul-Bob
Copy link
Contributor

Paul-Bob commented Oct 2, 2024

Cool! Looking good!

@enderahmetyurt enderahmetyurt linked a pull request Oct 2, 2024 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request Good first issue Good for newcomers hacktoberfest Help wanted We could use some help with this
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

4 participants