Comments (9)
Hi folks, sorry missed this. I really can't comment on this gem. If you find a problem with the dirty plugin on Mobility (without actiontext) then that's something I could maybe help, but then you'd need an isolated test to reproduce the issue.
from mobility-actiontext.
@csalmeida thank you for all your investigations. Unfortunately I have no time to dig in myself, but by all means if you find something that needs fixing please feel free to submit a PR. Also I would encourage trying to narrow down which part of the code is causing the issue, i.e. whether this should be solved in this gem or in Mobility itself. If it relates to Mobility code then @shioyama might offer advice.
from mobility-actiontext.
Thanks, @shioyama, this is an issue I would like to look into further, seems like an interesting problem.
Will create an application with the issue and try to suggest a solution as soon as I have a spare minute and share it here so you and @sedubois can take a look. Thanks again 🙏
from mobility-actiontext.
Hi @csalmeida, thanks for the report! As this gem does not explicitly touch the Dirty plugin, could you please check if you can reproduce the issue when not using mobility-actiontext
and only using the mobility
gem? In that case it will make more sense to open the issue on the Mobility repo. I would also suggest reporting the version of the plugins you're using.
from mobility-actiontext.
Hi @sedubois, thank you for getting back to me! I see, the reason I opened there is that this only happens to rich text objects but it could be better placed in Mobility.
It might be that I'm missing something but that's the behaviour I'm getting that happens when a field is set to use the action text backend a the moment.
I have a repository here where this could be replicated, sorry I could not get it deployed but I have invited you to it and it's public: https://github.com/csalmeida/rails-mobility
Creating a song and then editing it to check for the changed attribute should replicate the results I'm having.
I'm thinking that maybe because it's markup coming from Trix it might get modified in the client for some reason perhaps?
I have the following model using just Mobility's field definitions and submit a form without changing any fields I get:
Model definition:
class Song < ApplicationRecord
attr_accessor :language
extend Mobility
translates :title, type: :string, locale_accessors: [:en, :pt]
translates :description, type: :text, locale_accessors: [:en, :pt]
translates :content, type: :text, locale_accessors: [:en, :pt]
end
Inspecting after @song.update
it works as expected no values changed and therefore previous_changes
is empty:
(ruby) @song.previous_changes
{}
When submitting the form again but this time with a value changed it reflects that change in previous_changes
, sof far so good!
(ruby) @song.previous_changes
{"updated_at"=>[Sat, 10 Dec 2022 16:09:21.395697000 UTC +00:00, Sat, 10 Dec 2022 16:16:55.869677000 UTC +00:00], "title_en"=>["The wood of Maçaranduba", "The wood of Maçaranduba 2"]}
Now I'm introducing a Mobility Action Text field, and submitting the form without changing any values:
class Song < ApplicationRecord
attr_accessor :language
extend Mobility
translates :title, type: :string, locale_accessors: [:en, :pt]
translates :description, type: :text, locale_accessors: [:en, :pt]
translates :content, backend: :action_text, locale_accessors: [:en, :pt]
end
The input now becomes:
(Duration: 0.7ms | Allocations: 1072)
{"updated_at"=>[Sat, 10 Dec 2022 16:19:40.699785000 UTC +00:00, Sat, 10 Dec 2022 16:20:33.864672000 UTC +00:00],
"content_en"=>
[#<Mobility::Backends::ActionText::RichTextTranslation:0x0000000113f44bd8
id: 2,
name: "content",
body: #<ActionText::Content "<div class=\"trix-conte...">,
record_type: "Song",
record_id: 1,
created_at: Sun, 03 Apr 2022 22:25:54.450739000 UTC +00:00,
updated_at: Sun, 03 Apr 2022 23:27:54.407177000 UTC +00:00,
locale: "en">,
#<Mobility::Backends::ActionText::RichTextTranslation:0x0000000113f44bd8
id: 2,
name: "content",
body: #<ActionText::Content "<div class=\"trix-conte...">,
record_type: "Song",
record_id: 1,
created_at: Sun, 03 Apr 2022 22:25:54.450739000 UTC +00:00,
updated_at: Sun, 03 Apr 2022 23:27:54.407177000 UTC +00:00,
locale: "en">],
"content_pt"=>
[#<Mobility::Backends::ActionText::RichTextTranslation:0x0000000113f44a20
id: 3,
name: "content",
body: #<ActionText::Content "<div class=\"trix-conte...">,
record_type: "Song",
record_id: 1,
created_at: Sun, 03 Apr 2022 23:20:59.179068000 UTC +00:00,
updated_at: Sun, 03 Apr 2022 23:20:59.179068000 UTC +00:00,
locale: "pt">,
#<Mobility::Backends::ActionText::RichTextTranslation:0x0000000113f44a20
id: 3,
name: "content",
body: #<ActionText::Content "<div class=\"trix-conte...">,
record_type: "Song",
record_id: 1,
created_at: Sun, 03 Apr 2022 23:20:59.179068000 UTC +00:00,
updated_at: Sun, 03 Apr 2022 23:20:59.179068000 UTC +00:00,
locale: "pt">]}
If the title is changed that change is added to the changes array as well:
{"updated_at"=>[Sat, 10 Dec 2022 16:16:55.871974000 UTC +00:00, Sat, 10 Dec 2022 16:19:40.697910000 UTC +00:00],
"title_en"=>["The wood of Maçaranduba 2", "The wood of Maçaranduba"],
"content_en"=>
[#<Mobility::Backends::ActionText::RichTextTranslation:0x0000000113d97128
id: 2,
name: "content",
body: #<ActionText::Content "<div class=\"trix-conte...">,
record_type: "Song",
record_id: 1,
created_at: Sun, 03 Apr 2022 22:25:54.450739000 UTC +00:00,
updated_at: Sun, 03 Apr 2022 23:27:54.407177000 UTC +00:00,
locale: "en">,
#<Mobility::Backends::ActionText::RichTextTranslation:0x0000000113d97128
id: 2,
name: "content",
body: #<ActionText::Content "<div class=\"trix-conte...">,
record_type: "Song",
record_id: 1,
created_at: Sun, 03 Apr 2022 22:25:54.450739000 UTC +00:00,
updated_at: Sun, 03 Apr 2022 23:27:54.407177000 UTC +00:00,
locale: "en">],
"content_pt"=>
[#<Mobility::Backends::ActionText::RichTextTranslation:0x0000000113d96f20
id: 3,
name: "content",
body: #<ActionText::Content "<div class=\"trix-conte...">,
record_type: "Song",
record_id: 1,
created_at: Sun, 03 Apr 2022 23:20:59.179068000 UTC +00:00,
updated_at: Sun, 03 Apr 2022 23:20:59.179068000 UTC +00:00,
locale: "pt">,
#<Mobility::Backends::ActionText::RichTextTranslation:0x0000000113d96f20
id: 3,
name: "content",
body: #<ActionText::Content "<div class=\"trix-conte...">,
record_type: "Song",
record_id: 1,
created_at: Sun, 03 Apr 2022 23:20:59.179068000 UTC +00:00,
updated_at: Sun, 03 Apr 2022 23:20:59.179068000 UTC +00:00,
locale: "pt">]}
Could I be missing something here? It could well be that I'm assuming that previous changes should be empty in this particular case but maybe that's intended behaviour.
Thank you very much, I'm curious to read what you think. 🙏
from mobility-actiontext.
Found another important aspect, after the update the object has a different ID than the one it had before saving the record:
Before @song.update
:
#<Mobility::Backends::ActionText::RichTextTranslation:0x00000001192de460
id: nil,
name: "description",
body: nil,
record_type: "Song",
record_id: 4,
created_at: nil,
updated_at: nil,
locale: "en">
After @song.update
:
#<Mobility::Backends::ActionText::RichTextTranslation:0x000000011a3e6168
id: nil,
name: "description",
body: nil,
record_type: "Song",
record_id: 4,
created_at: nil,
updated_at: nil,
locale: "en">
Could it be that this is what ends up adding the value to the @song.previous_changes
hash?
It could also be related to the dirty
implementation as it patches methods so they work with translated attributes but I need to investigate further to be sure.
from mobility-actiontext.
In the meantime, I think I found a way to get past this, perhaps it's not the best solution but it works:
# app/controllers/application_controller.rb
# This method was created because Mobility Action Text objects are added incorrectly to changed objects.
# At the moment only Rails and Mobility attributes track the dirty property correctly.
def record_changed?(previous_record, current_record, params)
record_changes = current_record.previous_changes
mobility_action_text = "Mobility::Backends::ActionText::RichTextTranslation"
# Loop through each array of changes:
record_changes&.each do |key, value|
# Previous value is used to compare the class name but it can't be compared with changed_value because they're both storing updated values.
previous_value = value[0]
changed_value = value[1]
# Check if the items for each changes array are part of Mobility Action Text
if previous_value&.class&.name == mobility_action_text
# Compares a duplicate of the original object because previous_value and changed value cannot be trusted.
# Removes key entirely if the value is still the same:
if previous_record&.public_send(key)&.to_plain_text == current_record&.public_send(key)&.to_plain_text
record_changes.delete(key)
end
end
end
# If both content and description were not changed, then remove update from the hash.
if record_changes.size == 1 && record_changes["updated_at"]&.present?
record_changes.delete("updated_at")
end
# An array of parameters with nested values
return record_changes.empty? ? false : true
end
Using the method in a controller action:
# app/controllers/songs_controller.rb
def update
@song = Song.find_by(id: params[:id])
# Allows all tags are removed on submission.
# Otherwise it will the last one.
if params.dig(:song, :tag_ids).nil?
params[:song][:tag_ids] = []
end
duplicate_record = @song.dup
if @song.update(song_params)
log_contribution(@song) if record_changed?(duplicate_record, @song, song_params) # Save a contribution for this user if record changed.
redirect_to songs_path
else
render :edit
end
end
from mobility-actiontext.
For now, I've solved it like this, sharing in case it helps anyone in the future:
def record_changed?(previous_record, current_record, params)
record_changes = current_record.previous_changes
mobility_action_text = "Mobility::Backends::ActionText::RichTextTranslation"
# Loop through each array of changes:
record_changes&.each do |key, value|
# Previous value is used to compare the class name but it can't be compared with changed_value because they're both storing updated values.
previous_value = value[0]
changed_value = value[1]
# Check if the items for each changes array are part of Mobility Action Text
if previous_value&.class&.name == mobility_action_text
# Compares a duplicate of the original object because previous_value and changed value cannot be trusted.
# Removes key entirely if the value is still the same:
if previous_record&.public_send(key)&.to_plain_text == current_record&.public_send(key)&.to_plain_text
record_changes.delete(key)
end
end
end
# If both content and description were not changed, then remove update from the hash.
if record_changes.size == 1 && record_changes["updated_at"]&.present?
record_changes.delete("updated_at")
end
# Whether or not the record has changed.
return record_changes.empty? ? false : true
end
Please feel free to close if this is out of scope of the library, thanks a lot! 🙏
from mobility-actiontext.
@csalmeida thank you for all your investigations. Unfortunately I have no time to dig in myself, but by all means if you find something that needs fixing please feel free to submit a PR. Also I would encourage trying to narrow down which part of the code is causing the issue, i.e. whether this should be solved in this gem or in Mobility itself. If it relates to Mobility code then @shioyama might offer advice.
No worries at all I completely understand it might be difficult to find time to look into it. 🙏
It's an interesting problem and would be happy to look more into it as soon as I have a spare minute, I'll keep you both posted if I get to investigate further.
from mobility-actiontext.
Related Issues (10)
- Publish gem to rubygems.org HOT 3
- error when destroying record HOT 6
- Ruby minimum versions HOT 1
- with_rich_text_#{name} eager loading does not work HOT 5
- Require only needed Rails frameworks at runtime HOT 1
- Rendering locale specific fields with Mobility.normalize_locale(locale) is not supported. HOT 2
- Does this gem also need mobility to be installed as well? HOT 5
- Syntax for accessing records via query HOT 1
- No plugin configured for these keys: type. (Mobility::Pluggable::InvalidOptionKey) HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
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.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from mobility-actiontext.