1. Atlassian Labs
  2. Atlassian Labs
  3. bamboo-after-deployment-trigger
  4. Pull requests

Fixes issue #2 - eliminate getAllDeploymentProjects lookup with autocomplete against rest api

#2 Merged at 682d9b8
Repository
eddiewebb
Branch
issue-2-non-admin-performance
Repository
atlassianlabs
Branch
master
Author
  1. Eddie Webb
Reviewers
Description

Fixes issue #2 by eliminating getAllDeploymentProjects lookup with autocomplete against rest api

  • Uses bamboos quicksearch api to dynamically load deployment project as user types

  • removed deploymentProjects list from configurator class, but now passes existing name if set to pre-load dropdown on edit.

  • With these changes I saw no distinction between needs of ..ForEdit and ..ForView, so they both call the same shared methods to populate project ID & Name, and Environment list &ID

roundtrip.gif

(I also updated to bamboo 6.3 so I could use specs to pre-populate large volume of deploys, and eliminated fastdev/cli in favor of new quickreload approach, but neither is important to the functionality at hand)

  • Issues #2: Performance degradation for non-admin users resolved
  • Dependencies Checking for dependencies...
  • Dependents Checking for dependents...

Comments (8)

  1. Alexey Chystoprudov

    Hi @eddiewebb , thanks for your contribution. I'm happy to review this submission and get it merged into main repo, but I would need you to sign electronically the Contributor License Agreement (CLA) before I can accept this pull request. Go to https://developer.atlassian.com/opensource/ please, find the "Contributors License Agreement" section and fill appropriate CLA (individual in your case I suppose it would be) please.

  2. Eddie Webb author

    CLA signed. Also pushed import change you noted below (intellij automatically uses asterisk is more than a few are declared)

    1. Alexey Chystoprudov

      Thank you, I'll ask our legal department to confirm CLA signed before proceeding.

      Asterisk import is known issue, we have to change default IntelliJ configuration as part of onboarding process 🙂

  3. Alexey Chystoprudov

    I'll look at it on Monday, I need to test it locally but it looks like it search on build projects as well

    1. Eddie Webb author

      Yes, your right, it will. I can add JS side logic to filter it out while your legal team confirms. It still proved faster to use the lucene index than using the DeploymentProjectDao's string search.

      The validation errors when a build is picked (since it can't find environments) and also on save as invalid project ID.

      Edit . Fixed, see comment below

  4. Eddie Webb author

    HI @chystoprudov , I hope you found the additional filtering to eliminate non-deployment results suitable, please let me know if you have additional concerns.

    1. Alexey Chystoprudov

      @eddiewebb , sorry for delay. I'm on vacation till next week. Want to test if pagination works correctly when return back and accept this PR.

      1. Eddie Webb author

        @chystoprudov there is no pagination as it stands. Because we are using the quicksearch REST api we are limited to the single method exposed by https://docs.atlassian.com/atlassian-bamboo/6.3.1/com/atlassian/bamboo/index/quicksearch/QuickSearchSearcher.html which returns 12 results for the current term matching user permissions. (waits for at least 3 characters, searching every change thereafter)

        I took a look at leveraging the https://docs.atlassian.com/atlassian-bamboo/6.3.1/com/atlassian/bamboo/index/IndexerService.html but would end up duplicating so much of the code from quicksearcherimpl. The other option is to use the deploymentProjectDao and query directly against the DB, exposing our own summary results but both seemed to outweigh the benefit of infinite scrolling when the user can just keep modifying their entry to get a more targeted list

        Anyway, let me know your thoughts and I can poke down either path as you see fit.