[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
separate-translations-0.4 - hopefully fixes bugs in 070618-1
|
From: |
boud |
|
Subject: |
separate-translations-0.4 - hopefully fixes bugs in 070618-1 |
|
Date: |
Sat, 30 Jun 2007 04:30:24 +0200 (CEST) |
hi samizdat-devel,
On Tue, 26 Jun 2007, boud wrote:
hi samizdat-devel,
On Sun, 24 Jun 2007, boud wrote:
On Mon, 18 Jun 2007, Dmitry Borodaenko wrote:
Changes: translation is displayed in place of original in full mode
(when clicking to view the message) -- and the other way around,
original is listed and displayed in place of preferred translation,
all translations are dropped from replies list and are only available
through the list of translations in message info line, reply button is
not displayed on translation (no sense to enforce this latter rule
more strictly, as messages still can gain and loose status of
translation by changing the rating of their relation to Translation
focus).
Please give feedback, how does it compare to separate translation 0.2
patch?
The patch below to these 5 files:
models/message.rb
controllers/message_controller.rb
controllers/history_controller.rb
helpers/message_helper.rb
helpers/resource_helper.rb
should hopefully fix most of the bugs in the separate-translations
function in 070618-1. It's a patch against 070618-1.
The bugs in 070618-1 occur when the user's preferred language
(accept_language) is different to the language of the parent article.
This happens when trying to edit a parent or its translation.
The moderator functions are also messed up when accept_language is
different to the language of the parent article.
History also has some problems.
And it would be nice that the user uses "Add another focus" to add foci
to the parent, not to the translation.
i also think there are two principles we are trying for:
* the "naive" user who ignores resource id numbers should not be forced
to learn about them - s/he should be able to "just click and it works"
but:
* the "real" id of a resource, i.e. the one in the database, should be
used in URIs wherever possible and should match the content displayed
to the user, so as not to confuse the "advanced" user (or moderators),
except when a swap is necessary in order to display a translation.
Although this sounds simple, the various functions available are not fully
symmetrical, so it does get a bit complicated.
In the patch here, some bits of code are clearly repeated, so it should be
compactified at least a little, probably a lot.
cheers
boud
----------------------------------------------------------------------
--- /tmp/tmp_snapshot/samizdat/lib/samizdat/models/message.rb 2007-06-18
21:26:49.000000000 +0200
+++ /usr/lib/ruby/1.8/samizdat/models/message.rb 2007-06-26
01:15:05.000000000 +0200
@@ -158,6 +158,14 @@
@parent and @focuses.include?(Focus.translation_focus)
end
+ def is_parent_lang_preferred?(accept_language)
+ if @parent
+ parent = Message.cached(@parent)
+ parent.lang == accept_language
+ end
+ end
+
+
# find available translation to the most preferred language
#
# returns Message object or self, when no translation is suitable
@@ -168,7 +176,7 @@
if is_translation?
parent = Message.cached(@parent)
pt = parent.select_translation(accept_language)
- if pt.id == @id
+ if pt.id == @id
# if I'm the parent's preferred translation, swap us
return parent
end
--- /tmp/tmp_snapshot/samizdat/lib/samizdat/controllers/message_controller.rb
2007-06-18 13:01:02.000000000 +0200
+++ /usr/lib/ruby/1.8/samizdat/controllers/message_controller.rb
2007-06-30 02:25:37.070064384 +0200
@@ -33,16 +33,22 @@
@message = Message.cached(@id)
@message.assert_current_version
+ @t_id = (@message.is_parent_lang_preferred?(@request.accept_language)) ?
+ @message.id : @message.select_translation(@request.accept_language).id
+
+
if @request.has_key? 'confirm'
save('hide') do
@message.hide!(true)
end
- @request.redirect(@id)
+ (@message.is_translation?) ?
+ @request.redirect(@id) : @request.redirect(@t_id)
else
@title = _('Hide Message')
@content_for_layout = box(@title,
'<p class="moderation">' + _('The message will be hidden from public view.')
+ '</p>' +
- hide_message(Resource.new(@request, @id).short, true) +
+# hide_message(Resource.new(@request, @id).short, true) +
+ hide_message(Resource.new(@request, @t_id).short, true) +
secure_form(nil, [:submit, 'confirm', _('Confirm')]))
end
end
@@ -53,16 +59,26 @@
@message = Message.cached(@id)
@message.assert_current_version
+ @t_id = (@message.is_parent_lang_preferred?(@request.accept_language)) ?
+ @message.id : @message.select_translation(@request.accept_language).id
+
if @request.has_key? 'confirm'
save('unhide') do
@message.hide!(false)
end
- @request.redirect(@id)
+# (@message.is_translation?) ?
+# @request.redirect(@id) : @request.redirect(@t_id)
+
+ @t_id = (@message.is_parent_lang_preferred?(@request.accept_language)) ?
+ @message.id : @message.select_translation(@request.accept_language).id
+
+ @request.redirect(@t_id)
else
@title = _('Unhide Message')
@content_for_layout = box(@title,
'<p class="moderation">' << _('The message will not be hidden from public view.')
<< '</p>' <<
- Resource.new(@request, @id).short <<
+# Resource.new(@request, @id).short <<
+ Resource.new(@request, @t_id).short <<
secure_form(nil, [:submit, 'confirm', _('Confirm')]))
end
end
@@ -73,6 +89,9 @@
@message = Message.cached(@id)
@message.assert_current_version
+ @t_id = (@message.is_parent_lang_preferred?(@request.accept_language)) ?
+ @message.id : @message.select_translation(@request.accept_language).id
+
new_parent = @request['new_parent']
new_parent &&= normalize_reference(new_parent)
new_parent &&= @message.validate_reference(new_parent)
@@ -81,12 +100,18 @@
save('reparent') do
@message.reparent!(new_parent)
end
- @request.redirect(@message.id)
+
+ @t_id = (@message.is_parent_lang_preferred?(@request.accept_language)) ?
+ @message.id : @message.select_translation(@request.accept_language).id
+
+# @request.redirect(@message.id)
+ @request.redirect(@t_id)
else
@title = _('Reparent Message')
@content_for_layout = box(@title,
'<p class="moderation">' << _('This message will be moved to new parent') <<
'</p>' <<
- Resource.new(@request, @id).short <<
+# Resource.new(@request, @id).short <<
+ Resource.new(@request, @t_id).short <<
secure_form(nil,
[:label, 'new_parent', _('New Parent')],
[:text, 'new_parent', @message.parent],
@@ -171,7 +196,8 @@
@old_content.move_upload(@request, @id, version_id)
@message.content.move_upload(@request)
end
- @request.redirect(@message.id)
+# @request.redirect(@message.id)
+
@request.redirect(@message.select_translation(@request.accept_language).id)
elsif @request.has_key? 'preview'
preview
else
@@ -198,7 +224,13 @@
@old_content.move_upload(@request, @id, version_id)
@message.content.move_upload(@request)
end
- @request.redirect(@message.id)
+
+ @t_id = (@message.is_parent_lang_preferred?(@request.accept_language)) ?
+ @message.id : @message.select_translation(@request.accept_language).id
+
+# @request.redirect(@message.id)
+ @request.redirect(@t_id)
+
elsif @request.has_key? 'preview'
preview
else
@@ -227,7 +259,13 @@
# fixme: remove old file
@message.content.move_upload(@request)
end
- @request.redirect(@message.id)
+
+ @t_id = (@message.is_parent_lang_preferred?(@request.accept_language)) ?
+ @message.id : @message.select_translation(@request.accept_language).id
+
+# @request.redirect(@message.id)
+ @request.redirect(@t_id)
+
elsif @request.has_key? 'preview'
preview
else
--- /tmp/tmp_snapshot/samizdat/lib/samizdat/controllers/history_controller.rb
2007-04-16 21:13:09.000000000 +0200
+++ /usr/lib/ruby/1.8/samizdat/controllers/history_controller.rb
2007-06-30 03:20:45.294137880 +0200
@@ -15,12 +15,25 @@
resource = Resource.new(@request, @id)
- @title = _(resource.type) + ' / ' + resource.title + ' / ' + _('History of
Changes') +
+ if resource.type == 'Message'
+ message = Message.new(@id)
+ translation =
(message.is_parent_lang_preferred?(@request.accept_language)) ?
+ message : message.select_translation(@request.accept_language)
+ @t_id = translation.id
+ t_resource = Resource.new(@request, @t_id)
+ else
+ t_resource = resource
+ @t_id = @id
+ end
+
+ @title = _(resource.type) + ' / ' + t_resource.title + ' / ' + _('History
of Changes') +
+# @title = _(resource.type) + ' / ' + translation.title + ' / ' +
_('History of Changes') +
(skip > 0? sprintf(_(', page %s'), skip + 1) : '')
resource = nil
# current version
versions = (skip > 0? [] : [ @id ])
+# versions = (skip > 0? [] : [ @t_id ])
# previous versions
rdf.select_all( %{
@@ -42,7 +55,25 @@
# offer diff for last on page if not last in history
%{<a href="diff?old=#{last}&new=#{versions[i]}">#{compare}</a>}
end
- versions[i] = [ Resource.new(@request, versions[i]).list_item, diff_link
]
+
+ if 0 == i
+ resource = Resource.new(@request, versions[i])
+ if resource.type == 'Message'
+ message = Message.new(versions[i])
+ translation =
(message.is_parent_lang_preferred?(@request.accept_language)) ?
+ message : message.select_translation(@request.accept_language)
+ @t_id = translation.id
+ t_resource = Resource.new(@request, @t_id)
+ else
+ t_resource = resource
+ @t_id = @id
+ end
+
+ versions[i] = [ t_resource.list_item, diff_link ]
+ else
+ versions[i] = [ Resource.new(@request, versions[i]).list_item,
diff_link ]
+ end
+
end
versions.unshift [_('Versions'), _('Changes')]
--- /tmp/tmp_snapshot/samizdat/lib/samizdat/helpers/message_helper.rb
2007-06-18 21:19:43.000000000 +0200
+++ /usr/lib/ruby/1.8/samizdat/helpers/message_helper.rb 2007-06-30
03:28:29.086630704 +0200
@@ -42,15 +42,23 @@
_('guest') : %{<a
href="#{message.creator.id}">#{message.creator.full_name}</a>}
date = format_date(message.date)
+ t = (message.is_parent_lang_preferred?(@request.accept_language)) ?
+ message : message.select_translation(@request.accept_language)
+
if :full == mode
parent = %{<a href="#{message.parent}">} +
_('parent message') + '</a>' if message.parent
+ # TODO: probably s/message/t/ here too ?
version_of = %{<a href="#{message.version_of}">} + _('current version') +
'</a>' if message.version_of
- history = %{<a href="history/#{message.id}">} + _('history') +
- '</a>' if message.nversions.to_i > 0
- if format = SOURCE_FORMAT[message.content.format]
- source = %{<a href="message/#{message.id}/source" title="} +
+# history = %{<a href="history/#{message.id}">} + _('history') +
+# '</a>' if message.nversions.to_i > 0
+ history = %{<a href="history/#{t.id}">} + _('history') +
+ '</a>' if t.nversions.to_i > 0
+# if format = SOURCE_FORMAT[message.content.format]
+# source = %{<a href="message/#{message.id}/source" title="} +
+ if format = SOURCE_FORMAT[t.content.format]
+ source = %{<a href="message/#{t.id}/source" title="} +
_('view source') + %{">#{format}</a>}
end
end
@@ -72,7 +80,11 @@
Focus.new(@request, f, message.id)
})
- hidden = _('hidden') if message.hidden?
+# hidden = _('hidden') if message.hidden?
+ if ( translation and translation.hidden? ) or
+ ( not translation and message.hidden? )
+ hidden = _('hidden')
+ end
[ sprintf(_('by %s on %s'), creator, date.to_s),
parent, version_of, history, source, replies, translations, focuses,
hidden
@@ -135,6 +147,14 @@
buttons = ''
if not message.version_of # no buttons for old versions
+
+ # optimize: don't run select_translation 3 times here and in message()
+ # and in message_info
+# translation = (message.lang == @request.accept_language) ?
+# message : message.select_translation(@request.accept_language)
+ translation =
(message.is_parent_lang_preferred?(@request.accept_language)) ?
+ message : message.select_translation(@request.accept_language)
+
if @request.access('post')
unless message.is_translation?
# don't allow to post replies to translations
@@ -151,26 +171,27 @@
message.open
end
if open
- buttons << message_button(message.id, 'edit', _('Edit'))
+# buttons << message_button(message.id, 'edit', _('Edit'))
+ buttons << message_button(translation.id, 'edit', _('Edit'))
end
end
if @request.moderate?
buttons <<
message_button(
- message.id,
- message.hidden? ? 'unhide' : 'hide',
- message.hidden? ? _('UNHIDE') : _('HIDE'),
+ translation.id,
+ translation.hidden? ? 'unhide' : 'hide',
+ translation.hidden? ? _('UNHIDE') : _('HIDE'),
:moderator_action
) <<
- message_button(message.id, 'reparent', _('REPARENT'), :moderator_action)
<<
- message_button(message.id, 'takeover', _('TAKE OVER'), :moderator_action)
<<
- message_button(message.id, 'replace', _('REPLACE'),
:moderator_action)
+ message_button(translation.id, 'reparent', _('REPARENT'),
:moderator_action) <<
+ message_button(translation.id, 'takeover', _('TAKE OVER'),
:moderator_action) <<
+ message_button(translation.id, 'replace', _('REPLACE'),
:moderator_action)
end
elsif @request.moderate? # old versions can still be replaced
buttons <<
- message_button(message.id, 'replace', _('REPLACE'), :moderator_action)
+ message_button(translation.id, 'replace', _('REPLACE'),
:moderator_action)
end
('' == buttons) ? '' : %{<div class="foot">#{buttons}</div>\n}
@@ -184,7 +205,7 @@
#
def message(message, mode)
info = message_info(message, mode)
-
+
translation = (:preview == mode) ?
message :
message.select_translation(@request.accept_language)
@@ -216,7 +237,8 @@
#{title}<div class="info">#{info}</div>
<div class="content">#{content}</div>
</div>\n}
- hide_message(html, message.hidden?)
+# hide_message(html, message.hidden?)
+ hide_message(html, translation.hidden?)
end
# wrap message in a hidden-message div if it is hidden
--- /tmp/tmp_snapshot/samizdat/lib/samizdat/helpers/resource_helper.rb
2007-04-17 22:24:45.000000000 +0200
+++ /usr/lib/ruby/1.8/samizdat/helpers/resource_helper.rb 2007-06-30
03:44:11.234402472 +0200
@@ -42,13 +42,28 @@
}.join,
'focuses'
)
+
+ message = Message.new(related)
+ related_parent = (message.is_translation?) ?
+ message.parent : message.id
+
if @request.access('vote')
fbox << box(nil,
'<p><a class="action" title="' <<
_('Click to relate this resource to another focus') <<
- %{" href="resource/#{related}/vote">} <<
+ %{" href="resource/#{related_parent}/vote">} <<
_('Add another focus') <<
'</a></p>')
+ if config['calendar']
+ fbox << box(nil,
+ %{<p><a title="} <<
+ _('Click to put this resource in the calendar') <<
+ %{" href="resource/#{related_parent}/calendar_add">} <<
+ _('Put this in the calendar') << '</a></p>'
+ )
+ end # if config['calendar']
+
+
end
fbox
end
----------------------------------------------------------------------