Texuna Technologies fork at GitHub (continuation of #1650's issue-notes)
Added by Mischa The Evil almost 16 years ago
- Table of contents
- Description
- Fork investigation
- Context-menu action errors
- Your recommendations on #1650's discussion
- Other questions regarding changes in the fork
- Outro
Hello all,
Description¶
After starting discussion about Patch #1650 using it's issue-notes, I'll try to continue (and extend) discussion in this forum-thread hoping that it will become better traceable in future...
After I started discussion on patch #1650 Artem decided to publish Texuna Technologies'fork of Redmine at GitHub. Looking at it's README.rdoc I can see that it contains the following patches:- #1650 Start/end time tracking for timelogging
- #1671 Show a breakdown of estimated/spent/remaining time for a version
- #1717 Show diff for issue description change
- #1705 Use Rails timezones support
- #1680 Make version description multiline and textilizable
- #1676 Only show incomplete target versions
- massive Trac migration improvements
- User’s cross-project time log accessible from account page
- Don’t fill start date for new issue
I'll also use this thread to extend the discussion to changes made in the redmine-fork in general later.
Fork investigation¶
Finally, after quite busy period, I found some time to investigate the fork more closely. This took me a lot more time since I had some serious problems getting a clean diff against a export of Redmine core @ r1778. After I finally managed to get that clean diff I investigated all the changes closely.
I made quite detailed log of this process which shows what has changed in the files and why it changed.
I'll attach this log to this message for referencing the code changes. This comes in handy cause most of the changes are made in one single commit thus making it impossible to distinct changes between eachother.
See further "Other questions regarding changes in the fork"...
Context-menu action errors¶
Intro¶
Regarding my secondary discussion about patch #1650 which is about:
After a while of using my vendor-branch including this patch I found another issue regarding this patch... When using context-menu (right-click menu) in the issuelist, every change (even a more simpler priority-change) initiated through an action selected from the context-menu, brings me to the edit view for the issue showing me two errors saying: « Project » is invalid « Date » can't be blank In this view the requested changes are preselected. When I now click submit the change gets applied though without any problems. It looks like, if using the context-menu, those two parameters aren't passed to the controller somehow...
Behaviour of this error in your fork¶
While testing this with your fork (rebased against r1778) this still happens only while using the issuelist context-menu to change the issue's status.
When the default edit
method is used the described error starts to show up. This error is initially introduced after r1689 which adds timelog custom-fields.
It is that revision which adds a new line 163 to the edit-method in ../app/controllers/issues_controller.rb which looks like:
@time_entry = TimeEntry.new
Revision r1765 refactors the context-menu heavily. Before r1765 most actions uses the edit
-method. After r1765 most actions (except issue-status change [reason isn't clear to me at the moment??]) are using the bulk_edit
-method.
The bulk_edit
-method doesn't contain the line which is added to the edit
-method in r1689. This results in my conclusion that the bulk_edit
-method should be used for the issue-status change action in the context-menu.
- recieving the problems mentioned at first in the issue-notes of patch #1650
- recieving 0.0 timelog entries
Proposed workaround/fix¶
I found a way to solve/workaround this exact problem by using the bulk_edit
method (which is defined in source:trunk/app/controllers/issues_controller.rb#L218) instead of using the default (single issue edit) edit
method (which is defined in source:trunk/app/controllers/issues_controller.rb#L158) for the context-menu issue-status change links.
This change can be applied without any (unwanted) side-effects by changing the following loop in ../app/views/issues/context_menu.rhtml on lines 8-11 from:
<% @statuses.each do |s| -%>
<li><%= context_menu_link s.name, {:controller => 'issues', :action => 'edit', :id => @issue, :issue => {:status_id => s}, :back_to => @back}, :method => :post,
:selected => (s == @issue.status), :disabled => !(@can[:update] && @allowed_statuses.include?(s)) %></li>
<% end -%>
to
<% @statuses.each do |s| -%>
<li><%= context_menu_link s.name, {:controller => 'issues', :action => 'bulk_edit', :ids => @issues.collect(&:id), 'status_id' => s, :back_to => @back}, :method => :post,
:selected => (@issue && s == @issue.status), :disabled => !(@can[:update] && @allowed_statuses.include?(s)) %></li>
<% end -%>
I'd like to ask you to test this behaviour (and the proposed workaround/fix) and report your findings back into this forum-thread...
Your recommendations on #1650's discussion¶
Artem Vasiliev wrote:
Also you'll be welcome to try our other changes to Redmine, like diffs for issue descriptions (#1717) and remaining time calculation for version (#1671)
While disussing #1650 I already had those patched integrated into my private vendor-branch of Redmine. Both work fine and without any errors or problems.
Thanks for sharing those great patches... :-)
Other questions regarding changes in the fork¶
After investigating the diff between clean r1778 and this fork (changes upto 09/08/08) there are some questions raising in my head. I find some changes which are not clear to me why these changes are applied. This is also shown in the document I have attached which lists all the changes in your redmine-fork.
I'll list all changes which are unclear to me here hoping that you can provide the initial reason for the changes:
- add options to user authorisation:
- refactored options for inline images to always send mime-type?:
- add flash_error for issue:
- add flash_error for issues0:
- redirect after issue-move under some circumstances:
- apply_date addition (extra refinement?):
- check in find_project-method to make sure params[:project_id] !== empty:
- make sure apply_date is false when filter is all:
- make @from ||= Date.today - 1:
- make tO tO ||= Date.today:
- add: always guess and fill mime-type for attachments:
- clarified error when list custom_field list-type (?):
- use self.connection when rolling-back db transaction while bucking-out of an issue move using cancel:
- specified error-message to "project doesnt contain this tracker" hardcoded in the validate_on_create-method:
Outro¶
Now, I'll finish up this post. I hope it brings some light in the darkness regarding your great fork and the associated problems.
Greetings,
Mischa The Evil.
tt-master_changes.txt (13.4 KB) tt-master_changes.txt | Changes between tt_master-09/08/08 against clean trunk at r1778 |
Replies (6)
RE: Texuna Technologies fork at GitHub (continuation of #1650's issue-notes)
-
Added by Artem Vasiliev almost 16 years ago
Hi Mischa!
Thanks for a lot of work, in particular dissecting changes list from one big commit. I wish we used Git from beginning, and made commits to it from the start. Instead we commited to our local SVN repository, and I just didn't find clear way to move its commits to Git (moreover we would need to edit some of them before pushing to public because they had sensitive data like email addresses).
While testing this with your fork (rebased against r1778) this still happens only while using the issuelist context-menu to change the issue's status.
Strange, I've just tried changing issue Status through context menu on our instance - works fine, both with issues with timelog, without it, and 'in progress'. Did you try it with clean r1786 (now http://github.com/artemv/redmine_tt/tree/master is in sync with it)? As far as I understand you get the same error with it.
Best regards,
Artem
RE: Texuna Technologies fork at GitHub (continuation of #1650's issue-notes)
-
Added by Mischa The Evil almost 16 years ago
Hi there again,
Replies:¶
Artem Vasiliev wrote:
Thanks for a lot of work, in particular dissecting changes list from one big commit. I wish we used Git from beginning, and made commits to it from the start...
Your welcome :-) Would it maybe be possible for you to retrieve a svn log which shows which files were changed for what reason only? So without what's actually changed inside the files?
Such a log would really help understanding what's changed in that one big commit here in the GitHub-repo (thus automatically answering the questions I asked you here: http://www.redmine.org/boards/1/topics/show/2352#8)...
Artem Vasiliev wrote:
...need to edit some of them before pushing to public because they had sensitive data...
Good thing you think about that... Do a quick search on the net and you'll find lots of public-repos containing sensitive data (even root passes etc.) :evil:
Artem Vasiliev wrote:
Did you try it with clean r1786 (now http://github.com/artemv/redmine_tt/tree/master is in sync with it)? As far as I understand you get the same error with it.
After these suggestions you gave me I started another thorough "investigation" using several versions utilising a strict path which would show where the error gets triggered without suffering from environmental-differences. See my (again) extensive notes below...
Test:¶
Test Case¶
When using context-menu (right-click menu) in the issuelist, issue-status change initiated through an action selected from the context-menu, brings me to the edit view for the issue(s) showing me two errors saying:
« Project » is invalid « Date » can't be blank
In this view the requested changes are preselected. When I now click submit the change gets applied though without any problems.
Test Subjects¶
- Texuna Technologies'fork of Redmine (latest head of github-repo [commit: 177c5ac76913ad7576978f7946556b98392e741d] which is based (updated) to r1786
- Clean export of Redmine Core r1866 (latest head of Core-trunk, unpatched)
- Export of my vendor-branch (vendor-branch based on upstream's Core-trunk r1778 without proposed fix applied [http://www.redmine.org/boards/1/topics/show/2352#6] )
r1778 with proposed fix applied (http://www.redmine.org/boards/1/topics/show/2352#6] )
- Export of my vendor-branch (vendor-branch based on upstream's Core-trunk
Test Procedure¶
1.1. unzipped into redmine dir (with right permissions)
1.2. created db using create database redmine character set utf8;
1.3. edited db-settings according to 1.2.
- Downgraded the maximum-length of version-description to 255 (http://github.com/artemv/redmine_tt/tree/master/db/migrate/20080902122007_change_versions_description_limit.rb#L3) due to problems during migration (DB type-limit)
- Back-ported upstream's r1795 into TT-fork to get it working without 500 internal error (http://github.com/artemv/redmine_tt/tree/master/app/views/layouts/base.rhtml#L9)
1.4. rake db:migrate RAILS_ENV="production"
1.5. rake db:migrate_plugins RAILS_ENV="production"
1.6. rake redmine:load_default_data RAILS_ENV="production"
1.7. service httpd restart
2.1. created a public test-project named "great" (http://path.to.redmine/projects/add) using all defaults
2.2. added myself (admin) to a member of the project in the role of Manager
2.3. created a test-issue (http://path.to.redmine/projects/great/issues/new):
- Tracker: Bug
- Subject: TestIssue
- Description: TestIssue
- Status: New
- Priority: Normal
- Assigned to: - blank -
- Category: - blank -
- Start: - blank -
- Due date: - blank -
- Estimated time: - blank -
- % Done: - blank -
2.4. went to the issues-list (http://path.to.redmine/projects/great/issues)
2.5. alt + left-clicked (I'm using Opera) the newly created issue (which has current status "New") to open the context-menu
2.6. Selected "Status -> Assigned" (this is permitted within the default workflow loaded in 1.6)
Test Platform¶
Basic Info:¶
OS: | CentOS 4.4 [RHEL derivative] based Linux Distro, using back-ported kernel from CentOS 5 |
---|---|
Database: | MySQL 4.1.20 (distro-specific patched) |
Webserver: | Apache 2.0.52 (distro-specific patched) |
Ruby-Engine: | Ruby Enterprise Edition 1.8.6-20080709 |
Rails-Engine: | Phusion Passenger 2.0.2 (Rails 2.1.0) |
Output of ./script/about
:¶
About your application's environment Ruby version 1.8.6 (i686-linux) RubyGems version 1.2.0 Rails version 2.1.0 Active Record version 2.1.0 Action Pack version 2.1.0 Active Resource version 2.1.0 Action Mailer version 2.1.0 Active Support version 2.1.0 Application root /path/to/redmine Environment development (in fact production, but falsely reported due to Phusion Passenger implementation) Database adapter mysql Database schema version 20080914222030 (this differs between several tested subjects)
Test Results¶
----------------------------------------------------- | | Error Triggered? | ----------------------------------------------------- | TT-fork of Redmine (1) | YES | | Clean export of Redmine r1866 | NO | | My vendor-branch (2) | YES | | My vendor-branch (3) | NO | ----------------------------------------------------- (1) = latest head of github-repo (commit: "177c5ac76913ad7576978f7946556b98392e741d":http://github.com/artemv/redmine_tt/commit/177c5ac76913ad7576978f7946556b98392e741d) which is based (updated) to r1786 (To check this look at upstream's r1795; it isn't merged into TT's fork) (2) = vendor-branch based on upstream's r1778 without proposed fix applied (http://www.redmine.org/boards/1/topics/show/2352#6) (3) = vendor-branch based on upstream's r1778 with proposed fix applied (http://www.redmine.org/boards/1/topics/show/2352#6)
(BTW: I hope the TOC will be able to handle this thread... :-)
RE: Texuna Technologies fork at GitHub (continuation of #1650's issue-notes)
-
Added by Artem Vasiliev almost 16 years ago
Hi Mischa!
Wow. I'm going to try that procedure )
- Downgraded the maximum-length of version-description to 255 (http://github.com/artemv/redmine_tt/tree/master/db/migrate/20080902122007_change_versions_description_limit.rb#L3) due to problems during migration (DB type-limit)
Database: MySQL 4.1.20 (distro-specific patched)
Hm hm, we use MySQL 5.0.22 and it's runs that migration fine. I wonder can this test case be caused by using MySQL 4.1..
- Back-ported upstream's r1795 into TT-fork to get it working without 500 internal error (http://github.com/artemv/redmine_tt/tree/master/app/views/layouts/base.rhtml#L9)
Interesting, why r1795 exists and why it causes 500 error in your case.. Something specific to Passenger/sub-URI config?
Would it maybe be possible for you to retrieve a svn log which shows which files were changed for what reason only?
Yes, sure, gonna make it..
Best regards,
Artem
RE: Texuna Technologies fork at GitHub (continuation of #1650's issue-notes)
-
Added by Artem Vasiliev almost 16 years ago
- with FF instead of Opera;
- if the error persists, Mongrel (instead of Passenger). We use 1.1.5
- if the error persists, MySQL 5.0.22 instead of 4.1.20
Of cause the fact that you get the error with TT fork and doesn't on plain r1866 isn't pleasant to us; but at least let's dissect the environment bit it's reproduced in. To me it looks that most likely it's Opera or Passenger.
Best regards,
Artem
RE: Texuna Technologies fork at GitHub (continuation of #1650's issue-notes)
-
Added by Artem Vasiliev almost 16 years ago
Ok, here it is.. Though it's bit too big to be useful, especially without diffs )
tt-redmine-log.xml (398 KB) tt-redmine-log.xml |
RE: Texuna Technologies fork at GitHub (continuation of #1650's issue-notes)
-
Added by Mischa The Evil almost 16 years ago
Hello again,
After some testing i'm back again... :-)
1.
Downgraded the maximum-length of version-description to 255 (http://github.com/artemv/redmine_tt/tree/master/db/migrate/20080902122007_change_versions_description_limit.rb#L3) due to problems during migration (DB type-limit)
Database: MySQL 4.1.20 (distro-specific patched)
Hm hm, we use MySQL 5.0.22 and it's runs that migration fine. I wonder can this test case be caused by using MySQL 4.1..
This is certainly a thing off MySQL < 5.0.3.
Quote from MySQL-manual:
"Values in VARCHAR columns are variable-length strings. The length can be specified as a value from 0 to 255 before MySQL 5.0.3, and 0 to 65,535 in 5.0.3 and later versions."
2.
Back-ported upstream's r1795 into TT-fork to get it working without 500 internal error (http://github.com/artemv/redmine_tt/tree/master/app/views/layouts/base.rhtml#L9)
Interesting, why r1795 exists and why it causes 500 error in your case.. Something specific to Passenger/sub-URI config?
- r1795 exists because of:
- why it can't be a specific problem of my sub-URI setup at first is because I have ditched that setup. I switched to a setup which holds one app per subdomain (which is fact is a seperate apache vhost).
3.
Tried the procedure with our fork, got 'Successful update.' as usual.
So there must be a diff at some place... Hmm...
4.
Can you please try it:Ok, I've done the first two. I've now tried all combinations (client & server) using:
- with FF instead of Opera;
- if the error persists, Mongrel (instead of Passenger). We use 1.1.5
- if the error persists, MySQL 5.0.22 instead of 4.1.20
- Mozilla Firefox 2.0.0.16 / 3.0.1
- Opera 9.52
- Internet Explorer 7.0.5730.11
- Mongrel Gem 1.1.5
- Apache 2.0.52 with Phusion Passenger 2.0.2 (containing Rails 2.1.0)
but the error persists heavily in all combinations.
I can't test the third suggestion easily due to the fact that I'm bound to the latest (stable) version of MySQL provided by my distro-vendor (which btw is ClarkConnect 4.3). Besides that limitation I am not very certain that the cause of the issue is lying in the different MySQL-version. It seems a bit unlikely to me, though I won't deny it at this stage ;-)
5.
To me it looks that most likely it's Opera or Passenger.
Agreed! Though it doesn't seem to be the cause of my issue after some more testing with Mongrel and Firefox...
6.
Ok, here it is.. Though it's bit too big to be useful, especially without diffs
Thanks very much... Indeed: it's big. Though it'll certainly help me finding out (mainly using the "Search"-function of the editor) why some changes has been made (based on the commit-descriptions, so it actually all comes down to your clarity in the commit-description :-)
Round-up:
Of cause the fact that you get the error with TT fork and doesn't on plain r1866 isn't pleasant to us
Thanks for the "concern" :-) Though atm I've chosen to fix this issue using my proposed workaround (note: link is broken -> #6 is pointing to sixth item in first message of this thread).
The workaround seems to work fine for me without any side-effects.
This situation verses the amount of time I have (invested already) made me decide to stick to the workaround since it looks like I'm the only one who is/stays affected by this issue and the fact the the workaround is quite small and without side-effects.
If you still need some more info regarding the issue just let me know. I'm willing to test whatever when possible (in fact not-bounded due to distro-limitations).
Last thing:
Would you be so kind to take a(nother) quick look at these unclarities and then post a raw brain-dump (so without spending much time on it) showing your thoughts regarding them?
It would be really helpful to bind those unclarities to commits using the big log-file.... :-)
Thanks again for all help and support...
Greetings,
Mischa.