Horizon/Dashboard source reorganisation

Bug #1458697 reported by Richard Jones
16
This bug affects 3 people
Affects Status Importance Assigned to Milestone
OpenStack Dashboard (Horizon)
Fix Released
High
Richard Jones

Bug Description

The intent with Horizon has always been that the "horizon" component be a library/framework of components for dashboards to be built out of, and the "openstack_dashboard" component be an implementation of that.

For the purposes of succintness, "framework" in the below will refer to the folder horizon/horizon and "dashboard" will refer to "horizon/openstack_dashboard". Perhaps one day those could be renamed also...

Some reorganisation work has been done in bug https://bugs.launchpad.net/horizon/+bug/1454880 but it has focused on cleaning up the angularjs usage in our codebase.

During that work, several points were identified as blurring the line between framework and dashboard in the codebase, and this bug captures those. NOTE that this bug refers to the codebase *once that refactoring has been done* and leaf patch https://review.openstack.org/#/c/184597/ has been merged. Patches relating to this bug will all depend on that patch until it is merged.

Changes needing to be made:

1) the framework should not define the top-level angular app, and thus horizon/static/dashboard-app should go away, with parts moving to the dashboard and framework as appropriate.
2) the framework should also not define the top-level HTML (base.html) - this should be in the dashboard.

Actual steps to perform:

1) horizon/static/dashboard-app/utils should be moved to a new location in the framework. There's a large amount of tech debt in this folder which should be addressed in subsequent bugs and these files removed.
2) horizon/static/dashboard-app/login is a generic login/auth helper component should be moved to the framework.
3) horizon/static/dashboard-app/controllers has several controllers only used in the dashboard so should be moved there. The single controller "dummy.js" should remain in the framework.
4) horizon/static/dashboard-app/dashboard-app.module.js is the top-level angular app and should move to the dashboard.
5) horizon/templates/base.html should move to dashboard and its dependencies horizon/templates/horizon/_conf.html and horizon/templates/horizon/_scripts.html should move with it. There remains a need for a horizon/templates/horizon/_scripts.html in the framework for testing purposes, so a new file for that purpose will be placed in the testing area of the framework.

On the utils tech debt: the contents of that folder are only used in one place; the functions defined there should be moved to the modules they're used in. Those target modules are legacy code which will be replaced over time by angularjs implementations, so I believe it is reasonable to do this.

Changed in horizon:
assignee: nobody → Richard Jones (r1chardj0n3s)
status: New → In Progress
Revision history for this message
Rob Cresswell (robcresswell-deactivatedaccount) wrote :

This came out of the discussion at the summit. Prioritised and targeted for L1; important to resolve the organisation while it is still a relatively small codebase. Also a blocker on reliable documentation.

Changed in horizon:
importance: Undecided → High
milestone: none → liberty-1
Changed in horizon:
assignee: Richard Jones (r1chardj0n3s) → Thai Tran (tqtran)
Changed in horizon:
assignee: Thai Tran (tqtran) → Richard Jones (r1chardj0n3s)
Changed in horizon:
assignee: Richard Jones (r1chardj0n3s) → Thai Tran (tqtran)
Changed in horizon:
assignee: Thai Tran (tqtran) → Richard Jones (r1chardj0n3s)
Revision history for this message
Richard Jones (r1chardj0n3s) wrote :

Additional thoughts on the HTML move, based on discussion on email, my message there copied here verbatim:

"""
At the summit last week, we agreed to:

1. move base.html over from the framework to the dashboard, and
2. move the _conf.html and _scripts.html over as well, since they configure the application (dashboard).

Upon starting the work it occurs to me that all of the other files referenced by base.html should also move. So, here's the complete list of base.html components and whether they should move over in my opinion:

- horizon/_custom_meta.html
  Yep, is an empty file in horizon, intended as an extension point in dashboard. The empty file (plus an added comment) should move.
  - horizon/_stylesheets.html
  Is just a dummy in horizon anyway, should move.
- horizon/_conf.html
  Yep, should move.
- horizon/client_side/_script_loader.html
  Looks to be a framework component not intended for override, so we should leave it there.
- horizon/_custom_head_js.html
  Yep, is an empty file in horizon, intended as an extension point in dashboard. Move, with a comment added.
- horizon/_header.html
  There is a basic implementation in framework but the real (used) implementation is in dashboard, so should move.
- horizon/_messages.html
  This is a framework component, so I think should stay there. I'm not sure whether anyone would ever wish to override this. Also the bulk of it is probably going to be replaced by the <toast> implementation anyway... hmm...
- horizon/common/_sidebar.html
  This is an overridable component that I think should move.
- horizon/common/_page_header.html
  This is an overridable component that I think should move.
- horizon/_scripts.html
  Yep, should move.
"""

Revision history for this message
Richard Jones (r1chardj0n3s) wrote :

I have since changed my mind about horizon/common/_sidebar.html and horizon/common/_page_header.html - I think these perform much the same function as horizon/_messages.html and should stay in the framework, overridable in the dashboard.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to horizon (master)

Fix proposed to branch: master
Review: https://review.openstack.org/186295

Changed in horizon:
assignee: Richard Jones (r1chardj0n3s) → Ryan Peters (rjpeter2)
Changed in horizon:
assignee: Ryan Peters (rjpeter2) → Richard Jones (r1chardj0n3s)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to horizon (master)

Reviewed: https://review.openstack.org/185140
Committed: https://git.openstack.org/cgit/openstack/horizon/commit/?id=b388adce9330edd264537f5f5ab321b430d45fe5
Submitter: Jenkins
Branch: master

commit b388adce9330edd264537f5f5ab321b430d45fe5
Author: Richard Jones <email address hidden>
Date: Fri May 22 12:47:55 2015 -0700

    ngReorg - move utils from dashboad-app

    This will facilitate the removal of dashboard-app from
    horizon-the-framework.

    The "tech-debt" folder is intended to be removed in subsequent
    patches (still to be created) which move or remove the functions
    in that folder. The reason for not doing so now is that there
    would be functionality changes, and I wanted to keep this patch
    as small as possible.

    The dummy controller also moves to the same place as it
    is an empty controller that will eventually go away when
    the Django modal it's used in goes away.

    Change-Id: I0b89205b9c6c7b1498c1a3c48e53ba460b9e5469
    Partial-Bug: #1458697

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Reviewed: https://review.openstack.org/185133
Committed: https://git.openstack.org/cgit/openstack/horizon/commit/?id=cd50373ce0bc1b5d913a3443440928daebf79537
Submitter: Jenkins
Branch: master

commit cd50373ce0bc1b5d913a3443440928daebf79537
Author: Richard Jones <email address hidden>
Date: Fri May 22 13:05:30 2015 -0700

    ngReorg - move dashboard-app/login out

    This change will facilitate the removal of dashboard-app.

    Change-Id: Iab0f0c8018bc9bd2f8df124690da2316acb6c404
    Partial-Bug: #1458697

Changed in horizon:
assignee: Richard Jones (r1chardj0n3s) → Tyr Johanson (tyr-6)
Changed in horizon:
assignee: Tyr Johanson (tyr-6) → Ryan Peters (rjpeter2)
Changed in horizon:
assignee: Ryan Peters (rjpeter2) → Tyr Johanson (tyr-6)
Changed in horizon:
assignee: Tyr Johanson (tyr-6) → Travis Tripp (travis-tripp)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Reviewed: https://review.openstack.org/185132
Committed: https://git.openstack.org/cgit/openstack/horizon/commit/?id=742cb8a679be065aa43f688f941162714e84fb2e
Submitter: Jenkins
Branch: master

commit 742cb8a679be065aa43f688f941162714e84fb2e
Author: Richard Jones <email address hidden>
Date: Fri May 22 13:14:32 2015 -0700

    ngReorg - move dashboard controllers from horizon

    This change moves the controller ImageFormCtrl and
    hzNamespaceResourceTypeFormController from horizon (the framework)
    to openstack_dashboard (the application) where they belong.

    Co-Authored-By: Tyr Johanson <email address hidden>
    Change-Id: I2309f0867c2f657a6065003475dcb10448f907bc
    Partial-Bug: #1458697

Changed in horizon:
assignee: Travis Tripp (travis-tripp) → Ryan Peters (rjpeter2)
Changed in horizon:
assignee: Ryan Peters (rjpeter2) → Richard Jones (r1chardj0n3s)
Changed in horizon:
assignee: Richard Jones (r1chardj0n3s) → Ryan Peters (rjpeter2)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Reviewed: https://review.openstack.org/186295
Committed: https://git.openstack.org/cgit/openstack/horizon/commit/?id=fdce91b028bad12b14a816791dd05f84ea49bf83
Submitter: Jenkins
Branch: master

commit fdce91b028bad12b14a816791dd05f84ea49bf83
Author: Richard Jones <email address hidden>
Date: Thu May 28 14:40:21 2015 +1000

    ngReorg - move core HTML from framework to dashboard

    This patch moves the base.html file and related files (where appropriate)
    to the dashboard. In short:

    - base.html its *dashboard configuration* components _header.html,
      _stylesheets.html, horizon/_conf.html, horizon/_custom_head_js.html,
      horizon/_custom_meta.html and horizon/_scripts.html all move to the
      dashboard
    - other components in base.html which are effectively widgets or
      default content (horizon/_messages.html, horizon/common/_sidebar.html,
      horizon/common/_page_header.html,
      horizon/client_side/_script_loader.html) stay in the framework

    The framework test suite also includes a need for a base.html which can
    construct *part* of the application's base.html structure, and this is
    created in horizon/test/templates/base.html (this file is only used to
    test Django templating, so does not need most of the content in the real
    base.html).

    Change-Id: I9b3c4f21036d08e1d79aeab1ab35d47e3eedd3a3
    Partial-Bug: #1458697

Changed in horizon:
assignee: Ryan Peters (rjpeter2) → Richard Jones (r1chardj0n3s)
Changed in horizon:
status: In Progress → Fix Committed
Changed in horizon:
status: Fix Committed → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Reviewed: https://review.openstack.org/197310
Committed: https://git.openstack.org/cgit/openstack/horizon/commit/?id=ae9f5ce4b0802cc44383500eb0fc392a12fa31cd
Submitter: Jenkins
Branch: master

commit ae9f5ce4b0802cc44383500eb0fc392a12fa31cd
Author: Shaoquan Chen <email address hidden>
Date: Thu Jul 9 10:25:35 2015 -0700

    Dashboard ReOrg - Create app/core directory

    This patch creates the app/core directory. This will be the home
    for code that is application specific, but needed by multiple
    dashboards, such as the API files.

    Co-Authored-By: Shaoquan Chen <email address hidden>

    Change-Id: I93386a3b3bafe65f51eee72ff93283f1688de016
    Partially-Bug: #1458697

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Reviewed: https://review.openstack.org/197347
Committed: https://git.openstack.org/cgit/openstack/horizon/commit/?id=fede7c2a7e404d9447306857f403b227bbd8df60
Submitter: Jenkins
Branch: master

commit fede7c2a7e404d9447306857f403b227bbd8df60
Author: Shaoquan Chen <email address hidden>
Date: Thu Jul 9 15:10:09 2015 -0700

    Dashboard ReOrg - Move workflow into app/core

    This patch relocates the 'workflow' directory because
    it is application specific, but needed by multiple dashboards.

    Co-Authored-By: Shaoquan Chen <email address hidden>

    Change-Id: I0c2d8bb9bfd5ea14c2d970293ad2707e324fd5ab
    Partially-Bug: #1458697

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Reviewed: https://review.openstack.org/197334
Committed: https://git.openstack.org/cgit/openstack/horizon/commit/?id=e3e5ab834544a45c0729e2ca3861415552fd2264
Submitter: Jenkins
Branch: master

commit e3e5ab834544a45c0729e2ca3861415552fd2264
Author: Shaoquan Chen <email address hidden>
Date: Thu Jul 9 10:47:33 2015 -0700

    Dashboard ReOrg - Move cloud-services into app/core

    This patch relocates the 'cloud-services' directory because
    they are application specific, but needed by multiple dashboards.

    Co-Authored-By: Tyr Johanson <email address hidden>
    Co-Authored-By: Shaoquan Chen <email address hidden>

    Change-Id: Ief3b522ddeca2ea433c664c9b2ad61c245960812
    Partially-Bug: #1458697

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to horizon (master)

Fix proposed to branch: master
Review: https://review.openstack.org/203769

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on horizon (master)

Change abandoned by Shaoquan Chen (<email address hidden>) on branch: master
Review: https://review.openstack.org/203769
Reason: Abandoning since fixing app.module is prioritized.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to horizon (master)

Fix proposed to branch: master
Review: https://review.openstack.org/205720

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to horizon (master)

Reviewed: https://review.openstack.org/205720
Committed: https://git.openstack.org/cgit/openstack/horizon/commit/?id=66485be81a3f100b0672a3193823cd8a4bbc177e
Submitter: Jenkins
Branch: master

commit 66485be81a3f100b0672a3193823cd8a4bbc177e
Author: Tyr Johanson <email address hidden>
Date: Fri Jul 24 16:34:57 2015 -0600

    Auto-collect app/ instead of only app/core/

    Angular code for openstack_dashboard is almost finished
    migrating to openstack_dashboard/static/app/.

    Previously, the root module of app (app.module.js) was explicitly
    included from _conf.html so that environment variables from Django
    could be captured in the 'horizon.app.conf' constant.

    This prevented auto-collection at the static/app/ level because
    app.module.js will be included twice (once from _conf.html, and
    again during auto-collection)

    It turns out that 'horizon.app.conf' is unnecessary as an Angular
    constant. It's only use is to initialize the 'horizon.conf' global
    provided by horizon.js.

    This patch refactors _conf.html to directly initialize horizon.conf.
    This removes the need for app.module.js to be included from
    _conf.html, which allows us to change the auto-collection directory
    to 'app/'.

    Now all files in horizon/static and openstack_dashboard/static/app
    are auto-collected, without including app.module.js twice. All new
    angular files are expected to live in one of these two locations,
    minimizing changes to static_settings.py to include new files for
    auto-collection

    Change-Id: I95b6f13c85eb08c875885812a7db054a5aa6e7c2
    Partial-Bug: #1458697
    Closes-Bug: #1477825

Thierry Carrez (ttx)
Changed in horizon:
milestone: liberty-1 → 8.0.0
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.