#5502 Prevent adding certain tools multiple times

v1.1.0
closed
nobody
42cc (433)
General
2015-08-20
2012-12-20
Anonymous
No

Originally created by: vampire0

If you add the VHOST tool multiple times and then add a VHOST in one of the installed tools, it is also shown in the other tools, So as there is absolutely no benefit of adding the VHOST tool multiple times as far as I see, it should be prevented that it is added multiple times technically.
I think best would be if it is not in the list of addable tools if it is added already.

Related

Tickets: #5502
Tickets: #6321

Discussion

  • Dave Brondsema

    Dave Brondsema - 2013-01-02
    • summary: Prevent adding VHOST tool multiple times --> Prevent adding certain tools multiple times
    • milestone: limbo --> forge-backlog
     
  • Dave Brondsema

    Dave Brondsema - 2013-01-02

    I think it would be great if each tool App could have a max_instances property which could be set to 1, and prevent further installations. In the code it could also be a @property function and let it by dynamic.

     
  • Dave Brondsema

    Dave Brondsema - 2013-08-09

    [#6321] (duplicate) has further ideas on implementation

     

    Related

    Tickets: #6321

  • Chris Tsai - 2013-10-22

    Bump, I continue to get tickets from users that inadvertently add "Files" or other tools like that multiple times and then can't remove the extra.

    Then, we should take care of [#6320] for clean-up.

     
  • Dave Brondsema

    Dave Brondsema - 2013-10-22
    • labels: --> 42cc
     
  • Igor Bondarenko

    Igor Bondarenko - 2013-10-23
    • status: open --> in-progress
     
  • Igor Bondarenko

    Igor Bondarenko - 2013-10-23

    Created #462: [#5502] Prevent adding certain tools multiple times (3cp)

     

    Related

    Tickets: #5502

  • Igor Bondarenko

    Igor Bondarenko - 2013-11-28
    • status: in-progress --> code-review
     
  • Igor Bondarenko

    Igor Bondarenko - 2013-11-28

    Closed #462. {allura,forge-classic}:je/42cc_5502

    We was not able to find Apps for some tools mentioned in [#6321]. Here they are: mysql, prweb outgoing mail, vhost. I see that in sfx/prweb.py something like that used in AdminExtension, but I don't know how it can be installed, and therefore, how can they be limited. I think it's some kind of integration tools (with old sourceforge). Can you look at those? Everything else should work fine.

     

    Related

    Tickets: #6321

  • Dave Brondsema

    Dave Brondsema - 2013-12-03

    Yeah those were moved from apps to admin extensions.

     
  • Dave Brondsema

    Dave Brondsema - 2013-12-10
    • status: code-review --> in-progress
    • QA: Dave Brondsema
     
  • Dave Brondsema

    Dave Brondsema - 2013-12-10

    How about a real infinite limit as a default, instead of arbitrary 30? Then we don't break anywhere that is using more than 30 tools (it does happen...) Using float("inf") should work I think.

    The logic in installable_tools_for could use some fixing. The cls._installable_tools cache is global to the class, so can't have the per-project "limited_tools" filter applied to that data. Also the new code could be cleaner by using sets and/or comprehensions.

    It seems that when a tool has reached its max, it's no longer displayed as a choice, but there isn't server-side validation to enforce that. So you can open up /admin/tools twice and install a tool that has a max_instances = 1 in each window and both will succeed. Perhaps this is a bug I encountered only due to the issue I mention in the above paragraph?

    A few places were changed to check max_instances > 0 instead of installable I think they should still check installable in case an app wants to override that property and provide custom logic. def validate_mount_point and neighborhood_add_project.html specifically.

     
  • Igor Bondarenko

    Igor Bondarenko - 2013-12-18
    • status: in-progress --> code-review
     
  • Igor Bondarenko

    Igor Bondarenko - 2013-12-18

    Closed #505. Pushed to je/42cc_5502

    Looks like _installable_tools cache don't make sense at all, since it used only in installable_tools_for and recalculated on every call, so we've removed that. Also, limited_tools logic turns out unnecessary due to the way app.installable works.

     
  • Dave Brondsema

    Dave Brondsema - 2013-12-18
    • status: code-review --> closed
     

Log in to post a comment.