tessus / mwExtensionMantis

MediaWiki Extension:Mantis
https://www.mediawiki.org/wiki/Extension:Mantis
GNU General Public License v2.0
2 stars 6 forks source link

Allow using Description field as comment #34

Closed pedwik closed 5 years ago

pedwik commented 5 years ago

We are using this great extension to publish bug fix notes on a MediaWiki page, see for example https://www.heurekaslu.se/wiki/Version_2.12#Bug_fixes_and_other_changes. In addition to the Summary column in the table, we use the extension's Comment-feature to give a reader some more details on the bug. However, there is a bit of overhead work here, the content is often the same as that in the Description-field of the issue. Would it be possible to add functionality to support this? It would be controlled by an extension setting, for example, "use_description_field_as_comment=true/false". Existing comments should override this, for example some_bugid.comment = "some text", would not be replaced with that bud id:s description.

tessus commented 5 years ago

Yes, this should be possible.

I'm currently not able to work on it, but I will look into it in about 2 weeks.

tessus commented 5 years ago

@pedwik done.

The flag is called summary_as_comment. Set it to true and you're good to go.

P.S.: I still have to add it to the documentation. I guess I will do this tomorrow.

tessus commented 5 years ago

@pedwik just wanted to make sure you saw the update....

pedwik commented 5 years ago

Yes, it seems great and I look forward to try it asap (this week)!

chers, Peder

25 feb. 2019 kl. 18:59 skrev Helmut K. C. Tessarek notifications@github.com:

@pedwik just wanted to make sure you saw the update....

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

tessus commented 5 years ago

I've also updated the documentation: https://www.mediawiki.org/wiki/Extension:Mantis#summary_as_comment

pedwik commented 5 years ago

Hi Helmut, I have now installed the new version and it works perfectly. Thanks!

Best regards, Peder

Den mån 25 feb. 2019 kl 21:52 skrev Helmut K. C. Tessarek < notifications@github.com>:

I've also updated the documentation: https://www.mediawiki.org/wiki/Extension:Mantis#summary_as_comment

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tessus/mwExtensionMantis/issues/34#issuecomment-467178211, or mute the thread https://github.com/notifications/unsubscribe-auth/AMUIMzVWaZJO8Z1U25E6s6_4MkA00dcjks5vREz7gaJpZM4ahI0i .

tessus commented 5 years ago

Glad it works. Thanks for the feedback.

pedwik commented 5 years ago

Hello Helmut, I found a minor bug or unexpected behaviour today with the new "show summary as comment"-feature. If you do not enter any manual comment (such as 264.comment = "Some comment"), then the following warning occurs: Warning: Cannot modify header information - headers already sent by (output started at C:\Wiki\mw1291\extensions\Mantis\Mantis.php:1024) in ....

Best regards, Peder

Den tis 5 mars 2019 kl 17:28 skrev Helmut K. C. Tessarek < notifications@github.com>:

Glad it works. Thanks for the feedback.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tessus/mwExtensionMantis/issues/34#issuecomment-469746607, or mute the thread https://github.com/notifications/unsubscribe-auth/AMUIMxFtknSJS4X-gkz0hdWlcXl3VU1hks5vTpsmgaJpZM4ahI0i .

tessus commented 5 years ago

@pedwik I can't reproduce this problem.

Where do you see this warning? In your PHP error log? I don't get this warning. Also, is there more of the warning? Line number 1024 has nothing to do with output, it's an if statement.

What's even more puzzling: I do not send any headers in my extension. There's no single header() command in my code.

pedwik commented 5 years ago

Hello! I have activated the display of warnings and errors i localsettings.php: $wgShowExceptionDetails = true; $wgShowDBErrorBacktrace = true;

The warning is displayed in the browser [image: image.png]

Den fre 14 juni 2019 kl 20:28 skrev Helmut K. C. Tessarek < notifications@github.com>:

@pedwik https://github.com/pedwik I can't reproduce this problem.

Where do you see this warning? In your PHP error log? I don't get this warning. Also, is there more of the warning? Line number 1024 has nothing to do with output, it's an if statement.

What's even more puzzling: I do not send any headers in my extension. There's no single header() command in my code.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tessus/mwExtensionMantis/issues/34?email_source=notifications&email_token=ADCQQM5EZDRCOWDEG5MVL4TP2PPMLA5CNFSM4GUERURKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXXTLRQ#issuecomment-502216134, or mute the thread https://github.com/notifications/unsubscribe-auth/ADCQQM4FDNJCO2PFK7W2FCDP2PPMLANCNFSM4GUERURA .

tessus commented 5 years ago

I don't see the image (I only see [image: image.png]), but I've added the 2 parameters to my LocalSettings.php. I still do not get any warnings.

When you look at my code, the warning you describe makes no sense. Once again, my extension does not send any headers. So how can not setting a comment produce a warning that says "header already sent", if I do not send or modify any headers? (If you can answer this question, I am more than happy to spend time on this.) I understand that there could be some side effects (which still wouldn't make any sense, but they could be possible, if the mediawiki core code had a bug), but I can't even reproduce the warning.

I went through the code again and I couldn't see anything that would explain this warning.

I don't know what I can do here.

pedwik commented 5 years ago

Do you see the error if you visit the page? The full error message is displayed only when submitting the edits. https://www.heurekaslu.se/wiki/Version_2.13

Den fre 14 juni 2019 kl 21:17 skrev Helmut K. C. Tessarek < notifications@github.com>:

I don't see the image (I only see [image: image.png]), but I've added the 2 parameters to my LocalSettings.php. I still do not get any warnings.

When you look at my code, the warning you describe makes no sense. Once again, my extension does not send any headers. So how can not setting a comment produce a warning that says "header already sent", if I do not send or modify any headers? (If you can answer this question, I am more than happy to spend time on this.) I understand that there could be some side effects (which still wouldn't make any sense, but they could be possible, if the mediawiki core code had a bug), but I can't even reproduce the warning.

I went through the code again and I couldn't see anything that would explain this warning.

I don't know what I can do here.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tessus/mwExtensionMantis/issues/34?email_source=notifications&email_token=ADCQQM3GW3BPYDEARVCT3MTP2PVGDA5CNFSM4GUERURKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXXXFVQ#issuecomment-502231766, or mute the thread https://github.com/notifications/unsubscribe-auth/ADCQQM3CQOA3Q2YXN2CXOV3P2PVGDANCNFSM4GUERURA .

pedwik commented 5 years ago

Also the attached image.

Den fre 14 juni 2019 kl 21:17 skrev Helmut K. C. Tessarek < notifications@github.com>:

I don't see the image (I only see [image: image.png]), but I've added the 2 parameters to my LocalSettings.php. I still do not get any warnings.

When you look at my code, the warning you describe makes no sense. Once again, my extension does not send any headers. So how can not setting a comment produce a warning that says "header already sent", if I do not send or modify any headers? (If you can answer this question, I am more than happy to spend time on this.) I understand that there could be some side effects (which still wouldn't make any sense, but they could be possible, if the mediawiki core code had a bug), but I can't even reproduce the warning.

I went through the code again and I couldn't see anything that would explain this warning.

I don't know what I can do here.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tessus/mwExtensionMantis/issues/34?email_source=notifications&email_token=ADCQQM3GW3BPYDEARVCT3MTP2PVGDA5CNFSM4GUERURKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXXXFVQ#issuecomment-502231766, or mute the thread https://github.com/notifications/unsubscribe-auth/ADCQQM3CQOA3Q2YXN2CXOV3P2PVGDANCNFSM4GUERURA .

tessus commented 5 years ago

Apparently github does not send attachments via email. You probably would have to attach something in the UI. But no matter, I could see the message:

image

However, you will notice that the header does not come from my extension. It comes from WebResponse.php.

But I see something else at the bottom...

Ok, here is what is happening. My extension produces a PHP warning and the code which is hit when setting $wgShowExceptionDetails = true; screws up. This is clearly an issue with MediaWiki. So, you should not see the warning at the top of the page, only the warning at the bottom of the page.

When I fix the warning in my code, your other warning will be gone too, but that's not the real issue. I'd open a bug with mw core.

But thanks for reporting this. (Still not sure, why I did not see any warnings on my machine. Warnings are turned on.)

Can you try the following patch?

diff --git a/Mantis.php b/Mantis.php
index 3d8752c..dc6e8fc 100644
--- a/Mantis.php
+++ b/Mantis.php
@@ -1021,7 +1021,7 @@ function renderMantis( $input, $args, $mwParser )
                        $comment = trim(substr($row['summary'], 0, $conf['summarylength']))."...";
                    }
                }
-               if (array_key_exists($row['id'], $conf['comment']))
+               if ($conf['comment'] && array_key_exists($row['id'], $conf['comment']))
                {
                    $comment = $conf['comment'][$row['id']];
                }
tessus commented 5 years ago

PHP's warnings are garbage. I think the PHP devs have screwed up the design.

When your input is NULL, the output should be NULL or FALSE by design, but no, PHP additionally throws a warning.

tessus commented 5 years ago

@pedwik did you test the patch? does it fix your problem?

I've also created a new branch that includes the fix.

pedwik commented 5 years ago

Yes, it worked, thank you

Den sön 16 juni 2019 kl 23:37 skrev Helmut K. C. Tessarek < notifications@github.com>:

@pedwik https://github.com/pedwik did you test the patch? does it fix your problem?

I've also created a new branch https://github.com/tessus/mwExtensionMantis/tree/fix-warning that includes the fix.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tessus/mwExtensionMantis/issues/34?email_source=notifications&email_token=ADCQQMYVJEZUQGYTOXA7MR3P22XATA5CNFSM4GUERURKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXZVVXI#issuecomment-502487773, or mute the thread https://github.com/notifications/unsubscribe-auth/ADCQQM5PSHRBOTK7XV35CXLP22XATANCNFSM4GUERURA .

tessus commented 5 years ago

Thanks for the info. I will add the fix to master.

pedwik commented 5 years ago

The topic of this issue was to enable showing the issue description in the comment column of the mantis wiki table. I have mixed things up a bit in my mind, because setting "summary_as_comment" = true only means that you can show the summary field, which is already displayed if you have included 'summary' in the list of fields to include in the wiki table. To show the description the sql query must include a join to table bug_text_table.

tessus commented 5 years ago

Hmm, ok, so I misunderstood your original request. I thought you wanted the summary as comment, because with comments you could set a manual text as well (overriding the summary text). It was never possible to retrieve the description, mainly because it could be very long. Not sure how line breaks would be handled either.

I can have a look on Friday, but I'm not sure if this really is a good idea.