MantisBT

View Issue Details Jump to Notes ] Wiki ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0007835mantisbtfeaturepublic2007-03-16 18:062013-08-16 12:43
Reportergiallu 
Assigned To 
PrioritynormalSeverityminorReproducibilityhave not tried
StatusnewResolutionopen 
PlatformOSOS Version
Product Version 
Target VersionFixed in Version 
Summary0007835: Add "Has patch" flag to improve usefulness of the "Upload File" feature
DescriptionIt 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
Tagsattachments
Attached Filespng file icon Screenshot-NewAttachment.png [^] (36,068 bytes) 2007-03-16 18:06


zip file icon filehelper22_02_2008.zip [^] (4,586 bytes) 2008-02-22 10:47
zip file icon filehelper_stage1_29_02_2008.zip [^] (6,318 bytes) 2008-02-29 04:33
jpg file icon filehelper_stage1.JPG [^] (111,025 bytes) 2008-02-29 04:34
zip file icon filehelper_stage1_03_03_2008.zip [^] (5,818 bytes) 2008-03-03 02:37
zip file icon filehelper_stage1_03_03_2008_fixed.zip [^] (5,818 bytes) 2008-03-03 02:46
zip file icon filehelper_stage1b_06_03_2008.zip [^] (8,607 bytes) 2008-03-06 14:23
zip file icon filehelper_stage1_final1_11_03_2008.zip [^] (7,072 bytes) 2008-03-11 12:56
zip file icon filehelper_stage1_final2_26_03_2008.zip [^] (7,134 bytes) 2008-03-26 05:25
txt file icon filehelper_patch_versus_1.1.6_2009_05_28.txt [^] (23,744 bytes) 2009-05-28 07:50 [Show Content]

- Relationships
related to 0008134closed it's not possible to delete own attachments 
has duplicate 0008854closedgrangeway request: direct attachment replacement 
has duplicate 0003442closedvboctor Add description to uploading files 
has duplicate 0006603closeddregad Include descriptions with file attachments 
related to 0008068new Title of attachment isn't shown 
related to 0005009acknowledged Being able to know at what step a file was uploaded ? 
related to 0008287acknowledged Missing notification on file attachment 
related to 0008947new Reference to uploaded file in Notes 

-  Notes
User avatar (0014801)
vboctor (administrator)
2007-06-22 03:45

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.
User avatar (0014836)
giallu (developer)
2007-06-29 11:00

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
User avatar (0017154)
smig1o (reporter)
2008-02-22 10:48

Patch submitted.. it must be checked I think
User avatar (0017198)
giallu (developer)
2008-02-25 17:54

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
User avatar (0017201)
vboctor (administrator)
2008-02-26 03:16

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.
User avatar (0017206)
smig1o (reporter)
2008-02-27 07:06
edited on: 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.

User avatar (0017217)
smig1o (reporter)
2008-02-29 04:33

Ok. I think stage 1 is ready.
I post patch and screenshoot too.
<screenshoot>
* 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.
</screenshoot>
<changelog>
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_api.php
* 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
user_api- modified
strings_english and string_polish - added new strings
---
config_defaults_inc.php
$g_allow_mark_obsolete_threshold
$g_allow_mark_patch_threshold
added with comment
</changelog>

Someone should reviewe it and apply (I hope)
User avatar (0017238)
giallu (developer)
2008-03-02 19:33

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

any idea?
User avatar (0017240)
smig1o (reporter)
2008-03-03 02:48

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.
User avatar (0017241)
vboctor (administrator)
2008-03-03 02:55

That is what I hate about patches. I keep getting these hunk errors!
User avatar (0017242)
smig1o (reporter)
2008-03-03 03:33

Did you get today's fixed patch??
User avatar (0017245)
giallu (developer)
2008-03-03 05:09

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...
User avatar (0017272)
smig1o (reporter)
2008-03-06 14:23

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)

Enjoy..
User avatar (0017293)
vboctor (administrator)
2008-03-08 04:31

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.
User avatar (0017295)
smig1o (reporter)
2008-03-08 07:56

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 :)
User avatar (0017313)
smig1o (reporter)
2008-03-11 12:58

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.
User avatar (0021955)
emathieu (reporter)
2009-05-28 07:51

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.
User avatar (0037885)
atrol (developer)
2013-08-16 12:43

Removed assignment. giallu will not contribute to this issue in near future.

- Issue History
Date Modified Username Field Change
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


MantisBT 1.2.17 [^]
Copyright © 2000 - 2014 MantisBT Team
Time: 0.1237 seconds.
memory usage: 3,219 KB
Powered by Mantis Bugtracker