Jump to content
You're invited! Join our 4.6 Live Event on ZOOM 6/24 ×

Community

Colonel_mortis

+Clients
  • Posts

    1,380
  • Joined

  • Last visited

  • Days Won

    4

 Content Type 

Profiles

Downloads

IPS4 Providers

Release Notes

IPS4 Guides

IPS4 Developer Documentation

Invision Community Blog

Forums

Store

Posts posted by Colonel_mortis

  1. I've not really played around with achievements yet, but I think that's all the more reason to give this feedback now.

    My initial impression as a user on this site is that it's not obvious:

    • What points are
    • How I get points, and how many points things are worth
    • How many points I have
    • How the ranks compare (beyond just that each one is higher than the previous) (ie how many points each rank is worth)
    • What I can do to unlock new badges (there's an argument that some badges should be secret, but certainly most should not)

    These are the sorts of things that experienced members will figure out within a couple of weeks, and maybe all that information is available for users somewhere that I'm not immediately seeing. However, the fact that I'm not immediately seeing it is the problem. For achievements to really work they need to be walk-up usable - a new member needs to be incentivized to ask questions, to post good answers, and to come back to the site next week, and right now, it doesn't do that.

    On the user menu, it highlights my rank and progress, which is great, but I want to click it and see more details. It feels weirdly non-interactive, and leaves me wanting more info.

    For a good case study, take a look at Stack Overflow. I can look at all the badges, and see my progress towards them. I also see a graph of my rep (which on SO is less valuable because rep is compounding as people find your old questions, but on forums would be a more active incentive to keep increasing it).

    Building on the SO example, wouldn't it be neat if achievement ranks could be configured to actually benefit you? Even the group promotion rules are conspicuously missing the ability to promote by rank. I want to set some rules like "level 1 can only use forums, level 2 can use blogs and status updates, level 3 can post news, level 4 can use classifieds, level 5 gets more messenger storage, etc", which I think would be a great way to incentivise engagement and give users a goal to work towards that is more than just a slightly different coloured icon on their profile picture.

    Also, and this is borderline a bug, the default badge descriptions are inconsistent - some don't have descriptions, some have passive descriptions ("a week since joining"), some have present tense descriptions ("making your first post"), and some have past tense descriptions ("visited daily for a week"). There's also no way to edit them (which I will report as a bug).

    Another thing - wouldn't it be nice if getting a badge could give you points? As far as I can see, that's not a thing at the moment.

  2. When topics are merged, the view (manage) requests are redirected, but not any other controller methods, including not embeds. The redirects are really useful, but given that most links are actually in the form of embeds there's a significant gap there.

    There's also a lot of room for improvement on stopping people from destructively merging the wrong way on big topics (followers don't get ported across, and are irrecoverably deleted, maybe some other things too). The first post in the topic will always be the oldest, regardless of which was kept, and in almost every case I can think of, that means I would want to keep that post's title and therefore URL as well.

  3. I had a few complaints about it from my members too, and I ended up fixing it by changing the logic for sending a new notification to not send it if it's from an IP that they have used previously (except moderators and admins, which I retain the old behaviour). Since making that change, I haven't had any complaints from members.

    		if ( \IPS\Settings::i()->new_device_email && ($member->isAdmin() || $member->modPermission() ||
    				\IPS\Db::i()->select('count(*)', 'core_members_known_ip_addresses',
    					['member_id=? AND ip_address=?', $member->member_id, \IPS\Request::i()->ipAddress()]
    				)->first() === 0) )

     

  4. 5 hours ago, Nathan Explosion said:

    Let me break this down easily....

    If you quote this post then this quote will be discarded...

     

    But if you quoted this post then this quote wouldn't be discarded...

    Quote

    This content etc etc.

    @Colonel_mortis - feel free to step in if I am getting the wrong end of the stick here.

    Exactly

     

    5 hours ago, Jordan Invision said:

    AHHHH. Ok I see now. Sorry things got a little lost in translation. 😆 

    So we want to keep quotes in like this

    Quote

    To be or not to be. That is the question.

    But quotes that are replies to other members get discarded. 

    The first thing that comes to mind is changing the overall look of quotes like the one above, essentially offering two types of quotes:

    1. A reply to someone
    2. An actual quote

    Maybe the #2. looks like: 

    FpE6m.png

    Am I finally understanding it lolol? 

    Yeah, some sort of styling difference between the quote types might make sense. I think I'd want something that stands out slightly more than that, such as by adding a left border, such as stack overflow:

    image.png.82124d308df086787b563b9622baadab.png

  5. 8 minutes ago, Jordan Invision said:
    On 1/30/2021 at 10:47 PM, Colonel_mortis said:

    If I have a manually added quote (ie a quote added using the quote button rather than by quoting another post), it is often a relevant part of the actual post (especially when it's in the first post in a topic), so it would make sense for those quotes to remain as nested quotes when that post is quoted (especially now long quotes are truncated). It wouldn't be too hard to achieve - just look for the attribution attributes on the quote before stripping it.

    Nice suggestion, @Colonel_mortis

    Do you have any visual examples handy by chance? Something like this? 

    I was just thinking they should look something like this ^ (but only with manually added quotes, not "real" quotes as in this example). Having pull quotes like in your example is also an interesting idea, and one that I could maybe see some sites using, although my use case is people quoting news articles, for which I don't think that would be super useful.

  6. If I have a manually added quote (ie a quote added using the quote button rather than by quoting another post), it is often a relevant part of the actual post (especially when it's in the first post in a topic), so it would make sense for those quotes to remain as nested quotes when that post is quoted (especially now long quotes are truncated). It wouldn't be too hard to achieve - just look for the attribution attributes on the quote before stripping it.

  7. On 11/9/2020 at 3:12 PM, bfarber said:

    I've added your suggestions to an internal development suggestions list for consideration in an upcoming release.

    Another one I've just run into - \IPS\Content\Comment::contentImages calls $item->container() indiscriminately, which results in a bad method call exception. This can be triggered when viewing an activity stream RSS feed.

  8. When an ips.ui.editor instance is destroyed, you are following the answer from a Stack Overflow question and trying to manually destroy the CKE instance. Unfortunately, that manual destruction is not sufficient, and is leaving references to the editor and behaviour. Take ipsautosave as an example (and maybe the only example, I've not looked further) - it maintains a reference to `editor`, and runs autosave on it every two seconds. It attaches a `destroy` event handler, which would be triggered when calling editor.destroy(), to cancel the timeout and therefore release the editor reference, but because you never trigger the destroy event it's not sufficient. (I'm noticing this because I am trying to use multiple editors on a page with the same autosave key, but it's still a problem without that.) It looks like there are a few other references that destroy manually cleans up, so there are presumably a few other things that are leaked too.

    At a minimum, you should be doing `editor.fire("destroy")` to trigger all of that cleanup code, but ideally you should find the things that prevent editor.destroy() from working and fix those in CKE. As a middle ground, maybe you could do `try { editor.destroy(); } catch (e) { /* existing manual best effort */ }`.

  9. 7 hours ago, bfarber said:

    I fixed the DateInterval issue.

    I can't reproduce or see the first issue however, and my code lines aren't matching up.

    What do you have on line 1120?

    Ugh it's the patch from ticket #70349 again. Looking at the patch again it doesn't seem to break either of the things that I've run into, so it seems I somehow managed to misapply it (my linux git doesn't like that patch file at all, it's missing the a/ and b/ prefixes). Apologies for wasting your time twice for that.

  10. On 12/1/2020 at 2:58 AM, bfarber said:

    Let us know if you spot anything else.

    If you've not discovered it yet, I'm also getting

    TypeError: array_merge(): Argument #2 must be of type array, null given (0)

    in \IPS\forums\Forum::formatFormValues:1120 (neither $whereString nor $whereParams are defined as far as I and my IDE can tell, and $where seems to be unused).

     

    Also there are a few places where you have

    new\DateInterval(

    which seems to now be lexed as calling the function DateInterval in the namespace new, rather than applying the new operator to the root-namespace class DateInterval [1].

  11. 2 hours ago, RevengeFNF said:

    You noticed any change in terms of performance?

    I need to setup a test server for this.

    I've only been playing around with it IN_DEV so far, but I'm not noticing a difference. I don't think the JIT compiler is expected to make too much difference for web apps, but I would love to be wrong there.

  12. PHP 8.0.0 has been released. Some bugs that I've found in a couple of minutes of testing:

    • There are a bunch of deprecation warnings for required parameters following optional parameters - from my search with the regex
      function\s*\w+\s*\([^=)]*=[^)]*,\s*\$[^,)=]*[,)]
      there are 47 instances of this in the suite. It should be safe to remove all of the offending default parameters, since they can never be utilised.
    • The cms lang key can_edit_item_message_record is invalid - it contains %S rather than %s, which causes the page to (randomly?) 500.
    • Fatal error: Cannot make static method XMLReader::open() non static in class IPS\Xml\_XMLReader in .../system/Xml/XMLReader.php on line 34 (this might be a fun one to fix - it's a PHP 8 change that isn't compatible in either direction - https://www.php.net/manual/en/xmlreader.open.php#refsect1-xmlreader.open-changelog - you probably have to make a new method that delegates instead)
    • TypeError: method_exists(): Argument #1 ($object_or_class) must be of type object|string, null given from statusContainer.phtml:133 (when viewing a feed containing status updates)
    • PHP Fatal error:  Unparenthesized `a ? b : c ? d : e` is not supported. Use either `(a ? b : c) ? d : e` or `a ? b : (c ? d : e)` in .../applications/calendar/widgets/upcomingEvents.php on line 153

    If I come across any more issues, I'll add them in this topic, and encourage others to do the same.

  13. 10 hours ago, Matt said:

    You originally quoted 51% as false positives, what you do think it could be now?

    Trying to capture spam is a constantly evolving process. Trends wax and wane. We tweak things to account for this but we can only do so after a trend has established itself. Given the size of your community and the number of registrations you get, I'd love to know more about your data and look at our capturing system to increase its accuracy.

    Of the 147 members who were classified as 4 but not FASed, about 30 are likely spam accounts (based on manual classification by one of my moderators), which gives a precision of 61% (39% false positive rate) when classifying based on a score of 4.

    I'd be happy to share some more detailed data with you if there's anything that you think would be helpful - feel free to reach out by ticket/email/PM(/slack), whatever is easiest.

  14. Actually, looking further into the members who were caught by the spam service but weren't flagged as spammers, there are several who have made 0 posts (and thus weren't caught in my previous audit) but who are likely to be actual spammers based on their profile information. Based on the sample that I checked, the false positive rate is still too high to be useful, but it is not as high as I had originally thought.

  15. I run a large site, so I have a lot of legitimate members and receive a fairly significant number of spammers. I turned it off (but continued to collect members' spam scores) several months ago, due to the volume of requests by legitimate people who were being caught by the spam filter. I've run some statistics based on registrations over the past month, comparing whether they have been flagged as spammer (which is very closely correlated with whether they were actually a spammer (this may not be true - see my post later in the topic)) and the code that the IPS spam service gave. The results are:

    +-------+------+----------+
    | FASed | code | count    |
    +-------+------+----------+
    |     0 | 0    |       77 |
    |     0 | 1    |     5657 |
    |     0 | 2    |        3 |
    |     0 | 3    |        5 |
    |     0 | 4    |      147 |
    |     0 | null |      233 |
    |     1 | 0    |        3 |
    |     1 | 1    |      616 |
    |     1 | 2    |       13 |
    |     1 | 3    |       75 |
    |     1 | 4    |      155 |
    |     1 | null |        2 |
    +-------+------+----------+
    

    Breaking this down:

    • 12% of registrations are spammers
    • 6% of registrations receive a spam score > 1
    • 4% of registrations receive a spam score of 4

    So far, these numbers don't seem unreasonable. However,

    • If someone receives a spam score of 4 ("user is a known spammer"), they have 51% chance of actually being a spammer

    A precision of 51% is totally useless.

    • If someone is actually a spammer, they have a 28% chance of receiving a spam score > 1.

    That's a pretty shoddy recall too.

    If I set my site to reject members with a spam score of 4, I will lose ~150 members to the spam filter each month; even if I use 2 as the threshold instead it will still only reduce spam by 28%. That's not an OK trade-off to me.

    You may say that the answer to this is to set accounts to require admin validation instead when caught by the spam filter, and that's not unreasonable. However, I'm not logged into ACP every day, so I suspect this would result in the loss of a large portion of the potential registrations who will just go and ask their question on a different forum because they didn't want to wait. Furthermore, it's often not possible to tell whether the account is legitimate just based on the registration info, so that 28% hit rate is going to drop quite a lot more.

    I appreciate that catching spam is a very hard problem. However, I believe these number demonstrate that the current system is not fit for purpose, at least with the level of confidence that you currently assign to it ("member is a known spammer" in the config page is a long way from the truth, and "certain spammer" from your marketing materials is an outright lie).

  16. This was also brought up in the following topic, but there was no answer from IPS there and I do believe it is solvable:

    In almost all places where Item::$containerNodeClass is used, there is an isset test before accessing it and graceful fallback when it's not present. However, that check is missing in a few places that affect following and tagging.

    For followable:

    • \IPS\core\Followed\Table:99 - this join should actually be within the preceding if statement (since I believe it only makes sense when permissions are used)
    • core/front/table/tables/rows.phtml:104 - the if should also check that method_exists, as it the pattern everywhere else (but this isn't insurmountable because it will use the table desc if available)
    • core/front/tables/manageFollowRow.phtml:43 - as above
    • \IPS\Content\Search\Elastic\Query:467~487 - (what's going on with that code structure?) you should only honour $includeContainers if isset($class::$containerNodeClass)
    • \IPS\Content\Search\Mysql\Query:416~436 - as above

    for tags:

    • \IPS\Content\Item:7684 - should also check isset($containerClass)
    • \IPS\Content\Item:7727 - I think this should be a constant if containerClass is empty, but I'm not sure how it's used so I may be wrong here.

    To my eye, all those changes should be feasible, and would make it much easier to avoid nasty hacks involving pseudo-nodes. nb. I searched for these occurences using intellij, so there may be places that I missed because the static analysis wasn't powerful enough.

  17. On 10/27/2020 at 12:58 PM, Charles said:

    This only happens if your server is having problems processing an upgrade action in a reasonable amount of time. If it happens, you can always exit your browser and restart the upgrader. It will remember where it left off and try the proper way again.

    I have turned up the threshold for falling back to manual queries, because the vast majority of the time the queries that require manual action only take a couple of seconds to run (even with the threshold turned up). Maybe if I turn it down that will solve the issue, but instead it will mean I spend time copying queries into mysql-client instead. That raises another question though - can you fix the query runtime heuristic to be more accurate? UPDATE queries are pretty fast, as are DROP INDEX and I think DROP COLUMN, whereas CREATE INDEX queries are very slow, but they're all judged by the same threshold.

    Given the replies above though, as well as my memory of prior upgrades where I hadn't updated the threshold, resetting the threshold back to default is not the magic bullet here.

  18. If one request times out in the JS upgrader, it falls back to the considerably slower HTML-based upgrader. Could there not be some logic to retry the request a couple of times, ideally with a bit of backoff (if it's an nginx or transport layer error it's probably a timeout, and retrying will then hit the case where the task is already done), rather than making the upgrade experience suck? At worst, the HTML upgrader shouldn't have delays between redirects - as soon as the response comes back it is ready to receive another request.

    It's reasonably easy to work around by just deleting the mr parameter from the URL, but that relies on me paying attention to the upgrader.

  19. I don't say this much about this software, but the new ACP onboarding wizard is great. It has almost everything you need to bootstrap the community. However, it is missing settings to configure the ingoing and outgoing email addresses, and (less critically) the SMTP/etc settings, which means that emails default to being sent from the admin user's address (and therefore often don't get delivered).

  20. XRegExp is 135KB when minified as part of root_library.js (that whole bundle is only 383KB, so it's a significant chunk), but it's only used in two places:

    • In ipsautolink/plugin.js, it's just used to evaluate a native JS regex, which I believe is totally unnecessary (use this.urlRegex.test(text) instead!)
    • In ips.search.results.js, where it's just used to replace using a native JS regex (use $(this).text().replace(new RegExp(...), '...') instead, as you do for each subsequent replacement in that chain) (while you're at it, why does that code replace with HTML, HTML escape everything, then selectively unescape the content you just added?!)

    Removing those two places that don't, to my understanding, utilise the library in any meaningful way would allow you to slim down the site by a not-insignificant amount, and make certain people happy.

  21. Currently, there are no config options when following a member, so it's just binary following/not following. This means that you have to opt into all or nothing for being notified about new content posted by your followees - there's no way to follow a member for status updates only.

    It would make sense if the follow dialog when following a member let you choose between "status updates only" and "all content".

    Implementing this would be pretty straight forward I believe - just update the special casing of following members in \IPS\core\front\modules\system\notifications::follow to add another option for status updates only, then update the follow types checked when sending status notifications to include that type. Unfortunately the aforementioned follow method is basically unhookable because it constructs and uses the form in the same method without exposing it to hooks.

×
×
  • Create New...

Important Information

We use technologies, such as cookies, to customise content and advertising, to provide social media features and to analyse traffic to the site. We also share information about your use of our site with our trusted social media, advertising and analytics partners. See more about cookies and our Privacy Policy