@gparyani If it is a post that actually need to be reported, then sure, and you can even just report it here (and you generally should). Otherwise it may not be a good idea as it actually generates a MS report saying that you are reporting it with weight around 96.
@gparyani It depends on what you are wanting/needing to test. In general, I'd go with what @user12986714 said. The person who submitted the PR should have done or be doing any or all of the following: tested it on a test instance, created a unit test to verify the functionality, be watching or the next time it is used to verify that it's working. If you would like to specify what you want to verify is fixed, others can also be watching to verify.
If it's definitely necessary to test in a way that creates records of test posts on MS, then be sure to ping an MS developer so they can destroy the record of the post in order for it not to affect weights and other statistics.
Ah, I mistakenly thought that reporting in Charcoal Test wouldn't report on a "real" instance, as the Charcoal help pages said that room is for testing commands.
@Makyen As @gparyani said, the PR did not fix the issue. We probably need more logging to find out the underlying bug, as the code looks perfectly fine to me, and has not been changed since implemented. If it is really that piece of code causing the problem, it should not work at any time, but as we can see it just stopped working recently.
@gparyani Thank you for checking. Charcoal Test is generally for doing things that are noisy in chat and are not intended to affect normal operation. However, from SD's and MS' point of view, it's just like any other room in which SD is listening for commands.
@user12986714 I'm assuming that we're talking about the PR which changed adding a comment from a !!/report. Yes, I would not expect that PR to resolve the problem.
@user12986714 Since we can still post comments to MS using chat reply, it is also unlikely that send_auto_comment broke down. Hence my guess is that there are some problems with MS
@Makyen Also another problem is that prior to that PR, that functionality should not work as intended, since it passed comment instead of custom_reason. So even if it worked, the comment on MS should be something like "Post manually reported by user in room 1234 for reasons my_reason", instead of just "my_reason". However in the examples given, the latter is actually posted onto MS.
@Makyen So now it is a real mystery why it worked...
Maybe MS did some stripping on that side that gives out "my_reason"
Also that may be why the original implementation can pass comment unconditionally, without checking if the user actually used a custom reason (and expect MS to handle that)
@user12986714 I wouldn't expect MS to strip anything from the comment, but I would expect it to not create a comment if the length of the string is zero. I have not looked at the code recently enough to really comment on what happens past the code that was in the PR.
[ SmokeDetector | MS ] Bad ip for hostname in body, bad keyword in body, bad keyword with email in body, bad phone number in body, blacklisted website in body, +2 more (454): custom embroidered keychain by patchmaker01 on superuser.com
@Makyen well, your arguments on the allspam pr are quite valid. Since there are some random issues in report, and after viewing the code, the logic implemented was very unclear (and redundant) for me, my thoughts are to close that pr and refactor the entire report related functionalities altogether
Hopefully this can be done once I am using my computer again tomorrow
However problems are that if I am going to create another refdoc.md, it would conflict with that is in 3860. However if I don't, I can bet that 6 months later I would have zero idea about the logic I implemented (& the interface...)
@user12986714 That sounds like it's better in the long run. It really didn't look like there would be all that much that needs to be done to use report_posts() once the list of URLs is obtained, and would allow stripping out a bunch of code from allspam() that's redundant to the existing process of reporting posts.
@user12986714 One can always resolve merge conflicts. :;
@Makyen By the way I tried to add "raise ValueError("Hahaha")" at the beginning of report_posts, but when I ran the test locally, it won't actually raise the error (I can't see "ValueError: Hahaha" on my console)
@Makyen I literally just inserted that line under line 1787, and ran the test. It did give out some errors, but I can't see phrase "Hahaha" in the output (nor "ValueError")
@SmokeDetector Hopefully the new lock is not breaking things
@user12986714 It looks like everything within the new apiquota_rw_lock within bodyfetcher.py should be within a try/ except block that guarantees the lock is released if there's an exception. I'm not sure if we crash and reboot from in there or if we "recover". If we recover, then that lock needs to be released, or we're blocked. Hmmm... we should probably do the same thing for the existing api_request_lock, although it's partially covered.
Err also read GlobalVars.apiquota into current_apiquota after line 250 (of course with locking) and replace all GlobalVars.apiquota with current_apiquota. (except the assignment mentioned above)
@user12986714 It looks like there are other reasonable ways to do it. It would probably be better if the lock is held for a minimal amount of time. It could be optimized, but I'd already implemented the try/except..
@Makyen That pattern looks like it's already caught by Potentially bad keyword in body and Potentially bad keyword in answer; append -force if you really want to do that.
@Makyen my understanding is that we need to get line 244 in bodyfetcher deleted. (I'm not sure whether it is okay to double release locks though)
@Makyen I would play safe though. It is not any fun if chatcommunicate raises an exception and we are locked out forever.
Alright I'm sure now we need to get that line deleted. finally block is executed before return, and hence causing the lock being released twice. The latter release may release the lock another thread just acquired...
@Makyen No it won't. The lock doesn't know who is calling it, so if a thread acquired the lock prior to the second release, that lock is released. Now another thread may acquire that lock and create a huge mess by writing simultaneously
@tripleee That pattern looks like it's already caught by Potentially bad keyword in body and Potentially bad keyword in answer; append -force if you really want to do that.
[ SmokeDetector | MS ] Bad ip for hostname in body, bad keyword in body, bad keyword in title, blacklisted website in body, link at end of body, +4 more (789): Is Skincell Pro Canada by wiusds on askubuntu.com
@SmokeDetector Based on this answer and the answer for which they were blacklisted, my opinion is that they are here primarily to promote their newly posted blog entries, not to provide actual answers here on SE.
@Shree There's nothing that is expected to prevent that. Is the assumption that you used to be able to do so correct? What errors do you see in the console, or in the display?
[ SmokeDetector | MS ] Bad keyword in body, pattern-matching product name in body, potentially bad keyword in body, blacklisted user (266): Total Enhance RX by EmbellaCrea on askubuntu.com
@tripleee That pattern looks like it's already caught by Potentially bad keyword in body and Potentially bad keyword in answer; append -force if you really want to do that.
I really do feel it should be possible to detect the kind of word soup that this type of spam is. It is blatantly obvious for humans yet computers don't have a clue.
[ SmokeDetector | MS ] Bad keyword in answer, link at end of answer, potentially bad ip for hostname in answer, potentially bad ns for domain in answer, potentially bad keyword in answer, +2 more (281): Throttling requests in Cosmos DB SQL API by Training basket on stackoverflow.com
@Daniil That pattern looks like it's already caught by Potentially bad keyword in body and Potentially bad keyword in answer; append -force if you really want to do that.
Was that really spam? DRmare was already mentioned in the post. One of the suggested edit recently inserted the link for the same. I approved the edit thinking it as a legit edit.
@Kulfy there was an earlier and very obvious spam by the same user name a month ago, and several more further back, so pretty confident this was spam metasmoke.erwaysoftware.com/domains/21187
@tripleee That pattern looks like it's already caught by Blacklisted website in answer and Blacklisted website in body; append -force if you really want to do that.
