|View Issue Details|
|ID||Project||Category||View Status||Date Submitted||Last Update|
|0007835||mantisbt||feature||public||2007-03-16 18:06||2013-08-16 12:43|
|Priority||normal||Severity||minor||Reproducibility||have not tried|
|Target Version||Fixed in Version|
|Summary||0007835: Add "Has patch" flag to improve usefulness of the "Upload File" feature|
|Description||It would be useful to put an additional field for the "upload file" subsection, where the user can specify if the attached file is a patch for the issue.|
If we can implement this, along with the ability to query for bugs with patches
attached, we can give the proper priority to issues with a proposed fix attached.
Just reference, I am attaching the "Create New Attachment" page from
bugzilla, where I "borrowed" the idea
|I can't seem to open the PNG file. I like the idea, but I was thinking that we should support the concept of tags. Hence, an issue can be associated with multiple tags, one of these tags can be "patch". We can still have a check box on the file upload form that automatically associated the "patch" tag with the issue. This will also make filtering easier since it will build on whatever infrastructure we use for tags.|
The "tag" concept looks interesting (maybe it's to be discussed further on the ML?) but I think the "patch" property is something to be assigned to single attachments, not to the whole issue.
I will try to formalize what it's required for this in the wiki, then I will ask for comments on the ML
|Patch submitted.. it must be checked I think|
Thanks for the contribution. Here is my comments on the patch.
First of all, I note you based the patch on 1.1 code. Our guidelines for stable releases http://www.mantisbt.org/wiki/doku.php/mantisbt:development_scheme [^] does not allow us to add new features there, so you should port the patch to svn trunk.
That said, some technical stuff:
1. the schema update should be done in admin/schema.php
2. I think we can avoid having a new file bug_mark_obsolete.php the required logic should be added to bug_file_add.php
3. I'd like obsoleting a previous file while attaching the new one. This means:
3a: If current user is above "can_obsolete" threshold, make a checklist of _non obsolete_ attachments and allow me to select from them
3b. this should go in the same box where I upload the file (so it's handled by bug_file_add.php)
4. The old "Delete" link should be available as before, but with the mark_as_deleted meaning.
5. The Remove from DB action replaces the old "Delete" operation
For a first round I think it's enough... I also inquired about the status of print_bug_attachments_list() and it seems it is listed under Deprecated functions by mistake.
Thank you again
1. Relating to field names (e.g. ispatch), we typically don't have the "is" prefix for the boolean flags.
2. bug_mark_obsolete.php doesn't do access checking. This may go away based on giallu's comments. However, it is very important for action pages to re-validate the access level.
3. bug_mark_obsolete.php and similar pages should not include SQL queries. The scripts should call core APIs that execute the necessary sanitization and queries.
4. Are we assuming that reporters can always obsolete attachments and mark them as patches? This is not clear in the documentation in config_default_inc.php.
5. There is a couple of typos in the comments for $g_allow_mark_patch_threshold in config_defaults_inc.php.
6. "unpatch" and "unobsolete" - I don't think these terms make a lot of sense.
7. Would be nice to see screenshots of the GUI. Or if there is a demo instance available to try.
Last edited: 2008-02-27 07:06
Thanks for comments.
1. @giallu1 & @vboctor 1 - I've done changes in admin/schema.php. fileds names are patch and obsolete.
2. @giallu 2 & @vboctor 2 - logic moved to bug_file_add.php so all checks done in there. bug_mark_obsolete.php and such removed.
3. @vboctor 3 - new function added to file_api.php so now no SQL in GUI files.
4. @vboctor 4&5 - comments updated in config_default_inc.php. Typos.. well English isnt my prime language so im gonna need help with that.
5. @vboctor 6 - Ill change it in next patch.
6. @vboctor 7 - Ill try to submit a screenshoot when Ill finish with GUI, but need some more info from giallu for that.
Ok. I think stage 1 is ready.
I post patch and screenshoot too.
* File plik.exe is not submitted by current user and is market as obsolete (remove request). All info about the file is available.
* File 15.jpg us >regular file not submitted by current user
* File 00017972.jpg is patch file not submitted by current user.
* File 381520.gif if submitted by current user, so it may be market as a patch or as an obsolete.
* File baza.xls is submitted by current user. Its patch and its marked as obsolete.
Configuration doesnt allow current user to delete attachments. Even if he submitted it.
All was moved to >Attach file< box.
I had to add 3 fileds to mantis_bug_file_table:
* patch - curently as integer but should be smaller (smallint?? tinytiny??)
* obsolete - same as above
* user_id - id of user that submitted the file. It isnt foreign key mantis_user_table(id) since some files may be adandoned
I had to update bug_copy function in bug_api.php cos it contains SQL query instead of file_add function call.
* file_can_delete_bug_attachmets - now it checks >file submitter< vs >current user<
* file_add - added new fields to function definition
* file_can_mark_patch/obsolete - 2 functions that checks if current user can perform an action
* file_mark - sets/unsets file as patch/obsolete
* file_user_abandon - called from user_delete function
print_api - modified
strings_english and string_polish - added new strings
added with comment
Someone should reviewe it and apply (I hope)
I tried to apply the patch but failed:
[giallu@hal9001 mantisbt]$ patch -p2 < diffs
patching file admin/schema.php
patching file bug_file_add.php
patching file bug_file_upload_inc.php
patching file bug_view_advanced_page.php
patching file bug_view_page.php
patching file config_defaults_inc.php
Hunk 0000001 FAILED at 18.
patch: **** malformed patch at line 180: diff -Naur mantis_orig/mantisbt/core/bug_api.php mantis_rd/mantisbt/core/bug_api.php
No idea why.. if patch for config_defailts_inc.php in in the middle of the patch it fails. If I move it to the end.. its ok.
Im no diff expert...
The fixed file posted as last.
|That is what I hate about patches. I keep getting these hunk errors!|
|Did you get today's fixed patch??|
Well... hunk errors means something is not quite in sync between the reference revision the patch is based on and the revision you are applying the patch to.
it's sort of svn and cvs conflict state, so you definitely want to see them :)
In this case, it seems smig1o was handcrafting the patch file...
So here is updated patch with brand new attachments table.
I also did some sanitation (fixed db schema, fixed db calls)
Patch made against lastes source (5089)
A couple of comments:
1. Preview attachments seems to be broken (the table gets messed up, text is previewed in strange way, etc).
2. The report issue page doesn't seem to be updated yet.
3. We should add a field to the attachments that keep track of who uploaded the attachments.
1 @vboctor 2 Seems that I have forgotten about report page... sorru :)
2 @vboctor 3 Well mantis_bug_file_table has now >user_id< field with id of user that uploaded the file. So seems I dont understand that. Do you want to display >uploader< in file list too??
3 @vboctor 1 What do you mean >broken; messed up<?? I didnt noticed that... Maby some screenshoot so I could fix that. On the other hand.. Do we ned that preview?? If someone claims it broken/messed he can always turn it off...
Anyways I need more info about that to fix it.
thanks for feedback too :)
Ok added first >final< patch of the filehelper stage1. I hope its good enough to put it into mantis core.
patch and obsolete fields are now L type.
I found the patch very useful, thank you smig1o.
Since I had to make some changes to incorporate it into 1.1.6, I attached an updated patch based on this version.
|Removed assignment. giallu will not contribute to this issue in near future.|
|2007-03-16 18:06||giallu||New Issue|
|2007-03-16 18:06||giallu||File Added: Screenshot-NewAttachment.png|
|2007-06-22 03:45||vboctor||Note Added: 0014801|
|2007-06-22 03:45||vboctor||Status||new => acknowledged|
|2007-06-29 11:00||giallu||Note Added: 0014836|
|2007-07-08 17:43||giallu||Relationship added||related to 0008068|
|2007-07-11 06:38||giallu||Status||acknowledged => assigned|
|2007-07-11 06:38||giallu||Assigned To||=> giallu|
|2007-07-11 06:38||giallu||Relationship added||related to 0008134|
|2007-07-22 11:58||giallu||Relationship added||related to 0005009|
|2007-08-22 10:47||giallu||Relationship added||related to 0008287|
|2008-02-04 08:19||giallu||Relationship added||related to 0008854|
|2008-02-22 10:47||smig1o||File Added: filehelper22_02_2008.zip|
|2008-02-22 10:48||smig1o||Note Added: 0017154|
|2008-02-25 17:54||giallu||Note Added: 0017198|
|2008-02-26 03:16||vboctor||Note Added: 0017201|
|2008-02-27 07:06||smig1o||Note Added: 0017206|
|2008-02-27 07:06||smig1o||Note Edited: 0017206|
|2008-02-29 04:33||smig1o||Note Added: 0017217|
|2008-02-29 04:33||smig1o||File Added: filehelper_stage1_29_02_2008.zip|
|2008-02-29 04:34||smig1o||File Added: filehelper_stage1.JPG|
|2008-03-02 19:33||giallu||Note Added: 0017238|
|2008-03-03 02:37||smig1o||File Added: filehelper_stage1_03_03_2008.zip|
|2008-03-03 02:46||smig1o||File Added: filehelper_stage1_03_03_2008_fixed.zip|
|2008-03-03 02:48||smig1o||Note Added: 0017240|
|2008-03-03 02:55||vboctor||Note Added: 0017241|
|2008-03-03 03:33||smig1o||Note Added: 0017242|
|2008-03-03 05:09||giallu||Note Added: 0017245|
|2008-03-06 14:23||smig1o||Note Added: 0017272|
|2008-03-06 14:23||smig1o||File Added: filehelper_stage1b_06_03_2008.zip|
|2008-03-07 07:47||konstbel||Tag Attached: attachments|
|2008-03-08 04:31||vboctor||Note Added: 0017293|
|2008-03-08 07:56||smig1o||Note Added: 0017295|
|2008-03-11 12:56||smig1o||File Added: filehelper_stage1_final1_11_03_2008.zip|
|2008-03-11 12:58||smig1o||Note Added: 0017313|
|2008-03-13 23:44||vboctor||Relationship added||related to 0008947|
|2008-03-26 05:25||smig1o||File Added: filehelper_stage1_final2_26_03_2008.zip|
|2008-03-30 03:01||vboctor||Relationship added||has duplicate 0003442|
|2008-07-30 19:46||grangeway||Relationship replaced||has duplicate 0008854|
|2009-05-28 07:50||emathieu||File Added: filehelper_patch_versus_1.1.6_2009_05_28.txt|
|2009-05-28 07:51||emathieu||Note Added: 0021955|
|2012-03-21 06:13||dregad||Relationship added||has duplicate 0006603|
|2013-08-16 12:43||atrol||Note Added: 0037885|
|2013-08-16 12:43||atrol||Assigned To||giallu =>|
|2013-08-16 12:43||atrol||Status||assigned => new|