From d2b16aac7d6fe299648edbe7726d76ef49f52a51 Mon Sep 17 00:00:00 2001 From: wass3r <1301201+wass3r@users.noreply.github.com> Date: Fri, 2 Aug 2024 03:13:17 -0500 Subject: [PATCH 01/22] fix: reduce excessive API calls [wip] reduces api calls on build, services, graph, and repo pages avoids calls when builds are finished now with user agent --- src/elm/Api/Api.elm | 11 ++-- src/elm/Api/Header.elm | 20 ++++++- src/elm/Layouts/Default/Build.elm | 56 ++++++++++++++------ src/elm/Layouts/Default/Repo.elm | 10 +--- src/elm/Pages/Home_.elm | 2 +- src/elm/Pages/Org_/Repo_/Build_.elm | 39 ++++++++++---- src/elm/Pages/Org_/Repo_/Build_/Graph.elm | 37 +++++++++---- src/elm/Pages/Org_/Repo_/Build_/Services.elm | 46 ++++++++++++---- 8 files changed, 159 insertions(+), 62 deletions(-) diff --git a/src/elm/Api/Api.elm b/src/elm/Api/Api.elm index 6972e34ec..7bc103ded 100644 --- a/src/elm/Api/Api.elm +++ b/src/elm/Api/Api.elm @@ -17,6 +17,7 @@ module Api.Api exposing ) import Api.Endpoint as Endpoint exposing (Endpoint) +import Api.Header exposing (userAgent, userAgentString) import Api.Pagination as Pagination import Auth.Session exposing (Session(..)) import Http @@ -215,7 +216,7 @@ get : String -> Endpoint -> Decoder b -> Request b get api endpoint decoder = request { method = "GET" - , headers = [] + , headers = [ userAgent userAgentString ] , url = Endpoint.toUrl api endpoint , body = Http.emptyBody , decoder = decoder @@ -228,7 +229,7 @@ post : String -> Endpoint -> Http.Body -> Decoder b -> Request b post api endpoint body decoder = request { method = "POST" - , headers = [] + , headers = [ userAgent userAgentString ] , url = Endpoint.toUrl api endpoint , body = body , decoder = decoder @@ -241,7 +242,7 @@ put : String -> Endpoint -> Http.Body -> Decoder b -> Request b put api endpoint body decoder = request { method = "PUT" - , headers = [] + , headers = [ userAgent userAgentString ] , url = Endpoint.toUrl api endpoint , body = body , decoder = decoder @@ -254,7 +255,7 @@ delete : String -> Endpoint -> Decoder b -> Request b delete api endpoint decoder = request { method = "DELETE" - , headers = [] + , headers = [ userAgent userAgentString ] , url = Endpoint.toUrl api endpoint , body = Http.emptyBody , decoder = decoder @@ -267,7 +268,7 @@ patch : String -> Endpoint -> Decoder b -> Request b patch api endpoint decoder = request { method = "PATCH" - , headers = [] + , headers = [ userAgent userAgentString ] , url = Endpoint.toUrl api endpoint , body = Http.emptyBody , decoder = decoder diff --git a/src/elm/Api/Header.elm b/src/elm/Api/Header.elm index 6acf1d33a..77eae93b1 100644 --- a/src/elm/Api/Header.elm +++ b/src/elm/Api/Header.elm @@ -3,9 +3,10 @@ SPDX-License-Identifier: Apache-2.0 --} -module Api.Header exposing (get) +module Api.Header exposing (get, userAgent, userAgentString) import Dict exposing (Dict) +import Http {-| get : looks for the specified header key and returns the value. @@ -25,3 +26,20 @@ get key headers = |> Dict.fromList in Dict.get key_ headers_ + + +{-| userAgentString is the default User-Agent header value for the UI. + +TODO: pass this in via flags/config. + +-} +userAgentString : String +userAgentString = + "vela/ui" + + +{-| userAgent creates a User-Agent header with the specified value. +-} +userAgent : String -> Http.Header +userAgent value = + Http.header "user-agent" value diff --git a/src/elm/Layouts/Default/Build.elm b/src/elm/Layouts/Default/Build.elm index e4adaf2d0..4a06c54f1 100644 --- a/src/elm/Layouts/Default/Build.elm +++ b/src/elm/Layouts/Default/Build.elm @@ -328,24 +328,46 @@ update props shared route msg model = -- REFRESH Tick options -> + let + shouldRefresh = + case model.build of + RemoteData.Success build -> + build.finished == 0 + + _ -> + True + + runEffects = + if shouldRefresh then + [ Effect.getRepoBuildsShared + { pageNumber = Nothing + , perPage = Nothing + , maybeEvent = Nothing + , org = props.org + , repo = props.repo + } + , Effect.getBuild + { baseUrl = shared.velaAPIBaseURL + , session = shared.session + , onResponse = GetBuildResponse + , org = props.org + , repo = props.repo + , build = props.build + } + ] + + else + [ Effect.getRepoBuildsShared + { pageNumber = Nothing + , perPage = Nothing + , maybeEvent = Nothing + , org = props.org + , repo = props.repo + } + ] + in ( model - , Effect.batch - [ Effect.getRepoBuildsShared - { pageNumber = Nothing - , perPage = Nothing - , maybeEvent = Nothing - , org = props.org - , repo = props.repo - } - , Effect.getBuild - { baseUrl = shared.velaAPIBaseURL - , session = shared.session - , onResponse = GetBuildResponse - , org = props.org - , repo = props.repo - , build = props.build - } - ] + , Effect.batch runEffects ) diff --git a/src/elm/Layouts/Default/Repo.elm b/src/elm/Layouts/Default/Repo.elm index 753d7b4ed..e4192ca93 100644 --- a/src/elm/Layouts/Default/Repo.elm +++ b/src/elm/Layouts/Default/Repo.elm @@ -150,15 +150,7 @@ update props route msg model = Tick options -> ( model , Effect.batch - [ Effect.getCurrentUserShared {} - , Effect.getRepoBuildsShared - { pageNumber = Nothing - , perPage = Nothing - , maybeEvent = Nothing - , org = props.org - , repo = props.repo - } - , Effect.getRepoHooksShared + [ Effect.getRepoHooksShared { pageNumber = Nothing , perPage = Nothing , maybeEvent = Nothing diff --git a/src/elm/Pages/Home_.elm b/src/elm/Pages/Home_.elm index 1dd7646e2..972bac422 100644 --- a/src/elm/Pages/Home_.elm +++ b/src/elm/Pages/Home_.elm @@ -138,7 +138,7 @@ update msg model = -- REFRESH Tick options -> ( model - , Effect.getCurrentUserShared {} + , Effect.none ) diff --git a/src/elm/Pages/Org_/Repo_/Build_.elm b/src/elm/Pages/Org_/Repo_/Build_.elm index bd9d63a41..6cc20efa5 100644 --- a/src/elm/Pages/Org_/Repo_/Build_.elm +++ b/src/elm/Pages/Org_/Repo_/Build_.elm @@ -322,6 +322,7 @@ update shared route msg model = ( { model | steps = RemoteData.succeed <| List.sortBy .number steps } , steps |> List.filter (\step -> List.member step.number model.viewing) + |> List.filter (\step -> step.finished == 0) |> List.map (\step -> Effect.getBuildStepLog @@ -546,17 +547,35 @@ update shared route msg model = -- REFRESH Tick options -> + let + shouldRefresh = + case shared.builds of + RemoteData.Success builds -> + List.Extra.find (\b -> b.number == Maybe.withDefault 0 (String.toInt route.params.build)) builds + |> Maybe.map (\b -> b.finished == 0) + |> Maybe.withDefault False + + _ -> + False + + runEffect = + if shouldRefresh then + Effect.getBuildSteps + { baseUrl = shared.velaAPIBaseURL + , session = shared.session + , onResponse = GetBuildStepsRefreshResponse + , pageNumber = Nothing + , perPage = Just 100 + , org = route.params.org + , repo = route.params.repo + , build = route.params.build + } + + else + Effect.none + in ( model - , Effect.getBuildSteps - { baseUrl = shared.velaAPIBaseURL - , session = shared.session - , onResponse = GetBuildStepsRefreshResponse - , pageNumber = Nothing - , perPage = Just 100 - , org = route.params.org - , repo = route.params.repo - , build = route.params.build - } + , runEffect ) diff --git a/src/elm/Pages/Org_/Repo_/Build_/Graph.elm b/src/elm/Pages/Org_/Repo_/Build_/Graph.elm index 083f28cbd..291f53d61 100644 --- a/src/elm/Pages/Org_/Repo_/Build_/Graph.elm +++ b/src/elm/Pages/Org_/Repo_/Build_/Graph.elm @@ -20,6 +20,7 @@ import Http import Http.Detailed import Interop import Layouts +import List.Extra import Page exposing (Page) import Pages.Account.Login exposing (Msg(..)) import RemoteData exposing (RemoteData(..), WebData) @@ -237,6 +238,31 @@ update shared route msg model = ) Refresh options -> + let + shouldRefresh = + case shared.builds of + RemoteData.Success builds -> + List.Extra.find (\b -> b.number == Maybe.withDefault 0 (String.toInt route.params.build)) builds + |> Maybe.map (\b -> b.finished == 0) + |> Maybe.withDefault False + + _ -> + False + + runEffect = + if shouldRefresh then + Effect.getBuildGraph + { baseUrl = shared.velaAPIBaseURL + , session = shared.session + , onResponse = GetBuildGraphResponse { freshDraw = options.freshDraw } + , org = route.params.org + , repo = route.params.repo + , build = route.params.build + } + + else + Effect.none + in ( { model | graph = if options.setToLoading then @@ -246,14 +272,7 @@ update shared route msg model = model.graph } , Effect.batch - [ Effect.getBuildGraph - { baseUrl = shared.velaAPIBaseURL - , session = shared.session - , onResponse = GetBuildGraphResponse { freshDraw = options.freshDraw } - , org = route.params.org - , repo = route.params.repo - , build = route.params.build - } + [ runEffect , if options.clear then Effect.sendCmd clearBuildGraph @@ -357,7 +376,7 @@ subscriptions model = -- on visiblity changed, same as shared , Browser.Events.onVisibilityChange (\visibility -> VisibilityChanged { visibility = visibility }) - , Interval.tickEveryOneSecond Tick + , Interval.tickEveryFiveSeconds Tick ] diff --git a/src/elm/Pages/Org_/Repo_/Build_/Services.elm b/src/elm/Pages/Org_/Repo_/Build_/Services.elm index a483bb64b..a23b9a885 100644 --- a/src/elm/Pages/Org_/Repo_/Build_/Services.elm +++ b/src/elm/Pages/Org_/Repo_/Build_/Services.elm @@ -537,17 +537,43 @@ update shared route msg model = -- REFRESH Tick options -> + let + shouldRefresh = + case shared.builds of + RemoteData.Success builds -> + List.Extra.find (\b -> b.number == Maybe.withDefault 0 (String.toInt route.params.build)) builds + |> Maybe.map (\b -> b.finished == 0) + |> Maybe.withDefault False + + _ -> + False + + hasServices = + case model.services of + RemoteData.Success services -> + List.length services > 0 + + _ -> + False + + runEffect = + if shouldRefresh && hasServices then + Effect.getBuildServices + { baseUrl = shared.velaAPIBaseURL + , session = shared.session + , onResponse = GetBuildServicesRefreshResponse + , pageNumber = Nothing + , perPage = Just 100 + , org = route.params.org + , repo = route.params.repo + , build = route.params.build + } + + else + Effect.none + in ( model - , Effect.getBuildServices - { baseUrl = shared.velaAPIBaseURL - , session = shared.session - , onResponse = GetBuildServicesRefreshResponse - , pageNumber = Nothing - , perPage = Just 100 - , org = route.params.org - , repo = route.params.repo - , build = route.params.build - } + , runEffect ) From d724e0f1c0e6e9ab7b94951dff511a001401c22a Mon Sep 17 00:00:00 2001 From: wass3r <1301201+wass3r@users.noreply.github.com> Date: Fri, 2 Aug 2024 10:07:56 -0500 Subject: [PATCH 02/22] replace user agent with custom version header --- src/elm/Api/Api.elm | 12 ++++++------ src/elm/Api/Header.elm | 19 ++++++------------- 2 files changed, 12 insertions(+), 19 deletions(-) diff --git a/src/elm/Api/Api.elm b/src/elm/Api/Api.elm index 7bc103ded..57cbf8c4c 100644 --- a/src/elm/Api/Api.elm +++ b/src/elm/Api/Api.elm @@ -17,7 +17,7 @@ module Api.Api exposing ) import Api.Endpoint as Endpoint exposing (Endpoint) -import Api.Header exposing (userAgent, userAgentString) +import Api.Header exposing (velaVersionHeader) import Api.Pagination as Pagination import Auth.Session exposing (Session(..)) import Http @@ -216,7 +216,7 @@ get : String -> Endpoint -> Decoder b -> Request b get api endpoint decoder = request { method = "GET" - , headers = [ userAgent userAgentString ] + , headers = [ velaVersionHeader ] , url = Endpoint.toUrl api endpoint , body = Http.emptyBody , decoder = decoder @@ -229,7 +229,7 @@ post : String -> Endpoint -> Http.Body -> Decoder b -> Request b post api endpoint body decoder = request { method = "POST" - , headers = [ userAgent userAgentString ] + , headers = [ velaVersionHeader ] , url = Endpoint.toUrl api endpoint , body = body , decoder = decoder @@ -242,7 +242,7 @@ put : String -> Endpoint -> Http.Body -> Decoder b -> Request b put api endpoint body decoder = request { method = "PUT" - , headers = [ userAgent userAgentString ] + , headers = [ velaVersionHeader ] , url = Endpoint.toUrl api endpoint , body = body , decoder = decoder @@ -255,7 +255,7 @@ delete : String -> Endpoint -> Decoder b -> Request b delete api endpoint decoder = request { method = "DELETE" - , headers = [ userAgent userAgentString ] + , headers = [ velaVersionHeader ] , url = Endpoint.toUrl api endpoint , body = Http.emptyBody , decoder = decoder @@ -268,7 +268,7 @@ patch : String -> Endpoint -> Decoder b -> Request b patch api endpoint decoder = request { method = "PATCH" - , headers = [ userAgent userAgentString ] + , headers = [ velaVersionHeader ] , url = Endpoint.toUrl api endpoint , body = Http.emptyBody , decoder = decoder diff --git a/src/elm/Api/Header.elm b/src/elm/Api/Header.elm index 77eae93b1..7d86da564 100644 --- a/src/elm/Api/Header.elm +++ b/src/elm/Api/Header.elm @@ -3,7 +3,7 @@ SPDX-License-Identifier: Apache-2.0 --} -module Api.Header exposing (get, userAgent, userAgentString) +module Api.Header exposing (get, velaVersionHeader) import Dict exposing (Dict) import Http @@ -28,18 +28,11 @@ get key headers = Dict.get key_ headers_ -{-| userAgentString is the default User-Agent header value for the UI. +{-| velaVersionHeader creates a User-Agent header with the specified value. -TODO: pass this in via flags/config. +TODO: allow this to be controlled via flags/config -} -userAgentString : String -userAgentString = - "vela/ui" - - -{-| userAgent creates a User-Agent header with the specified value. --} -userAgent : String -> Http.Header -userAgent value = - Http.header "user-agent" value +velaVersionHeader : Http.Header +velaVersionHeader = + Http.header "x-vela-ui-version" "vela/ui" From b9bbcbc3fae7c1ffc3515c6a5d22066c72ec08e8 Mon Sep 17 00:00:00 2001 From: davidvader Date: Fri, 2 Aug 2024 12:17:33 -0500 Subject: [PATCH 03/22] fix: rerender graph every 1s --- src/elm/Pages/Org_/Repo_/Build_/Graph.elm | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/elm/Pages/Org_/Repo_/Build_/Graph.elm b/src/elm/Pages/Org_/Repo_/Build_/Graph.elm index 291f53d61..88338aec7 100644 --- a/src/elm/Pages/Org_/Repo_/Build_/Graph.elm +++ b/src/elm/Pages/Org_/Repo_/Build_/Graph.elm @@ -356,9 +356,16 @@ update shared route msg model = -- REFRESH Tick options -> - ( model - , Effect.sendMsg <| Refresh { freshDraw = False, setToLoading = False, clear = False } - ) + case options.interval of + Interval.FiveSeconds -> + ( model + , Effect.sendMsg <| Refresh { freshDraw = False, setToLoading = False, clear = False } + ) + + Interval.OneSecond -> + ( model + , Effect.sendCmd <| renderBuildGraph shared model { freshDraw = False } + ) @@ -377,6 +384,7 @@ subscriptions model = , Browser.Events.onVisibilityChange (\visibility -> VisibilityChanged { visibility = visibility }) , Interval.tickEveryFiveSeconds Tick + , Interval.tickEveryOneSecond Tick ] From 6bac33fc19d52a0a768a5df0c6a7d1a9ce1bf026 Mon Sep 17 00:00:00 2001 From: wass3r <1301201+wass3r@users.noreply.github.com> Date: Fri, 2 Aug 2024 17:54:45 -0500 Subject: [PATCH 04/22] remove breaking version header addition going to save this for a non-patch update --- src/elm/Api/Api.elm | 11 +++++------ src/elm/Api/Header.elm | 12 +----------- 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/src/elm/Api/Api.elm b/src/elm/Api/Api.elm index 57cbf8c4c..6972e34ec 100644 --- a/src/elm/Api/Api.elm +++ b/src/elm/Api/Api.elm @@ -17,7 +17,6 @@ module Api.Api exposing ) import Api.Endpoint as Endpoint exposing (Endpoint) -import Api.Header exposing (velaVersionHeader) import Api.Pagination as Pagination import Auth.Session exposing (Session(..)) import Http @@ -216,7 +215,7 @@ get : String -> Endpoint -> Decoder b -> Request b get api endpoint decoder = request { method = "GET" - , headers = [ velaVersionHeader ] + , headers = [] , url = Endpoint.toUrl api endpoint , body = Http.emptyBody , decoder = decoder @@ -229,7 +228,7 @@ post : String -> Endpoint -> Http.Body -> Decoder b -> Request b post api endpoint body decoder = request { method = "POST" - , headers = [ velaVersionHeader ] + , headers = [] , url = Endpoint.toUrl api endpoint , body = body , decoder = decoder @@ -242,7 +241,7 @@ put : String -> Endpoint -> Http.Body -> Decoder b -> Request b put api endpoint body decoder = request { method = "PUT" - , headers = [ velaVersionHeader ] + , headers = [] , url = Endpoint.toUrl api endpoint , body = body , decoder = decoder @@ -255,7 +254,7 @@ delete : String -> Endpoint -> Decoder b -> Request b delete api endpoint decoder = request { method = "DELETE" - , headers = [ velaVersionHeader ] + , headers = [] , url = Endpoint.toUrl api endpoint , body = Http.emptyBody , decoder = decoder @@ -268,7 +267,7 @@ patch : String -> Endpoint -> Decoder b -> Request b patch api endpoint decoder = request { method = "PATCH" - , headers = [ velaVersionHeader ] + , headers = [] , url = Endpoint.toUrl api endpoint , body = Http.emptyBody , decoder = decoder diff --git a/src/elm/Api/Header.elm b/src/elm/Api/Header.elm index 7d86da564..1a4243534 100644 --- a/src/elm/Api/Header.elm +++ b/src/elm/Api/Header.elm @@ -3,7 +3,7 @@ SPDX-License-Identifier: Apache-2.0 --} -module Api.Header exposing (get, velaVersionHeader) +module Api.Header exposing (get) import Dict exposing (Dict) import Http @@ -26,13 +26,3 @@ get key headers = |> Dict.fromList in Dict.get key_ headers_ - - -{-| velaVersionHeader creates a User-Agent header with the specified value. - -TODO: allow this to be controlled via flags/config - --} -velaVersionHeader : Http.Header -velaVersionHeader = - Http.header "x-vela-ui-version" "vela/ui" From e7310d56e036acf9435c243a55a5b5a0d10d14b8 Mon Sep 17 00:00:00 2001 From: wass3r <1301201+wass3r@users.noreply.github.com> Date: Fri, 2 Aug 2024 17:55:18 -0500 Subject: [PATCH 05/22] remove unused import --- src/elm/Api/Header.elm | 1 - 1 file changed, 1 deletion(-) diff --git a/src/elm/Api/Header.elm b/src/elm/Api/Header.elm index 1a4243534..6acf1d33a 100644 --- a/src/elm/Api/Header.elm +++ b/src/elm/Api/Header.elm @@ -6,7 +6,6 @@ SPDX-License-Identifier: Apache-2.0 module Api.Header exposing (get) import Dict exposing (Dict) -import Http {-| get : looks for the specified header key and returns the value. From e602332bffcc1115f65b8a5874bfc492143066ec Mon Sep 17 00:00:00 2001 From: wass3r <1301201+wass3r@users.noreply.github.com> Date: Fri, 2 Aug 2024 18:18:18 -0500 Subject: [PATCH 06/22] use steps status instead of build status to refresh steps see https://github.com/go-vela/ui/pull/104 --- src/elm/Pages/Org_/Repo_/Build_.elm | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/elm/Pages/Org_/Repo_/Build_.elm b/src/elm/Pages/Org_/Repo_/Build_.elm index 6cc20efa5..97cd4ab7a 100644 --- a/src/elm/Pages/Org_/Repo_/Build_.elm +++ b/src/elm/Pages/Org_/Repo_/Build_.elm @@ -548,18 +548,16 @@ update shared route msg model = -- REFRESH Tick options -> let - shouldRefresh = - case shared.builds of - RemoteData.Success builds -> - List.Extra.find (\b -> b.number == Maybe.withDefault 0 (String.toInt route.params.build)) builds - |> Maybe.map (\b -> b.finished == 0) - |> Maybe.withDefault False + isAnyStepActive = + case model.steps of + RemoteData.Success steps -> + List.any (\s -> s.finished == 0) steps _ -> False runEffect = - if shouldRefresh then + if isAnyStepActive then Effect.getBuildSteps { baseUrl = shared.velaAPIBaseURL , session = shared.session From 10cd55bc5cc7650f89b8034eab2e9b7d1b928787 Mon Sep 17 00:00:00 2001 From: wass3r <1301201+wass3r@users.noreply.github.com> Date: Fri, 2 Aug 2024 18:54:25 -0500 Subject: [PATCH 07/22] prevent doubling up on hooks call when on hooks/audit page --- src/elm/Layouts/Default/Repo.elm | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/src/elm/Layouts/Default/Repo.elm b/src/elm/Layouts/Default/Repo.elm index e4192ca93..8a7c45850 100644 --- a/src/elm/Layouts/Default/Repo.elm +++ b/src/elm/Layouts/Default/Repo.elm @@ -148,16 +148,25 @@ update props route msg model = -- REFRESH Tick options -> + let + shouldRefreshHooks = + route.path /= Route.Path.Org__Repo__Hooks { org = props.org, repo = props.repo } + + runEffect = + if shouldRefreshHooks then + Effect.getRepoHooksShared + { pageNumber = Nothing + , perPage = Nothing + , maybeEvent = Nothing + , org = props.org + , repo = props.repo + } + + else + Effect.none + in ( model - , Effect.batch - [ Effect.getRepoHooksShared - { pageNumber = Nothing - , perPage = Nothing - , maybeEvent = Nothing - , org = props.org - , repo = props.repo - } - ] + , runEffect ) From 560685e18c6be1e8f4336f993c00819d798ff5b6 Mon Sep 17 00:00:00 2001 From: wass3r <1301201+wass3r@users.noreply.github.com> Date: Fri, 2 Aug 2024 19:04:06 -0500 Subject: [PATCH 08/22] check if any service is running to determine whether to update instead of checking build status --- src/elm/Pages/Org_/Repo_/Build_/Services.elm | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/src/elm/Pages/Org_/Repo_/Build_/Services.elm b/src/elm/Pages/Org_/Repo_/Build_/Services.elm index a23b9a885..393d5df59 100644 --- a/src/elm/Pages/Org_/Repo_/Build_/Services.elm +++ b/src/elm/Pages/Org_/Repo_/Build_/Services.elm @@ -538,26 +538,16 @@ update shared route msg model = -- REFRESH Tick options -> let - shouldRefresh = - case shared.builds of - RemoteData.Success builds -> - List.Extra.find (\b -> b.number == Maybe.withDefault 0 (String.toInt route.params.build)) builds - |> Maybe.map (\b -> b.finished == 0) - |> Maybe.withDefault False - - _ -> - False - - hasServices = + isAnyServiceRunning = case model.services of RemoteData.Success services -> - List.length services > 0 + List.any (\s -> s.status == Vela.Running) services _ -> False runEffect = - if shouldRefresh && hasServices then + if isAnyServiceRunning then Effect.getBuildServices { baseUrl = shared.velaAPIBaseURL , session = shared.session From f08df4d8c875020ea7309340456fabd0df505a88 Mon Sep 17 00:00:00 2001 From: wass3r <1301201+wass3r@users.noreply.github.com> Date: Fri, 2 Aug 2024 19:06:00 -0500 Subject: [PATCH 09/22] match name with change made in service refresh logic --- src/elm/Pages/Org_/Repo_/Build_.elm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/elm/Pages/Org_/Repo_/Build_.elm b/src/elm/Pages/Org_/Repo_/Build_.elm index 97cd4ab7a..1a2e26b33 100644 --- a/src/elm/Pages/Org_/Repo_/Build_.elm +++ b/src/elm/Pages/Org_/Repo_/Build_.elm @@ -548,7 +548,7 @@ update shared route msg model = -- REFRESH Tick options -> let - isAnyStepActive = + isAnyStepRunning = case model.steps of RemoteData.Success steps -> List.any (\s -> s.finished == 0) steps @@ -557,7 +557,7 @@ update shared route msg model = False runEffect = - if isAnyStepActive then + if isAnyStepRunning then Effect.getBuildSteps { baseUrl = shared.velaAPIBaseURL , session = shared.session From ce7bec9f8f2c8c545d3d37b397a559849c41c3eb Mon Sep 17 00:00:00 2001 From: wass3r <1301201+wass3r@users.noreply.github.com> Date: Sat, 3 Aug 2024 17:54:50 -0500 Subject: [PATCH 10/22] minor refactor build layout refresh --- src/elm/Layouts/Default/Build.elm | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/src/elm/Layouts/Default/Build.elm b/src/elm/Layouts/Default/Build.elm index 4a06c54f1..9c1aef9bd 100644 --- a/src/elm/Layouts/Default/Build.elm +++ b/src/elm/Layouts/Default/Build.elm @@ -329,7 +329,7 @@ update props shared route msg model = -- REFRESH Tick options -> let - shouldRefresh = + isBuildRunning = case model.build of RemoteData.Success build -> build.finished == 0 @@ -337,15 +337,18 @@ update props shared route msg model = _ -> True + getRepoBuildsEffect = + Effect.getRepoBuildsShared + { pageNumber = Nothing + , perPage = Nothing + , maybeEvent = Nothing + , org = props.org + , repo = props.repo + } + runEffects = - if shouldRefresh then - [ Effect.getRepoBuildsShared - { pageNumber = Nothing - , perPage = Nothing - , maybeEvent = Nothing - , org = props.org - , repo = props.repo - } + if isBuildRunning then + [ getRepoBuildsEffect , Effect.getBuild { baseUrl = shared.velaAPIBaseURL , session = shared.session @@ -357,14 +360,7 @@ update props shared route msg model = ] else - [ Effect.getRepoBuildsShared - { pageNumber = Nothing - , perPage = Nothing - , maybeEvent = Nothing - , org = props.org - , repo = props.repo - } - ] + [ getRepoBuildsEffect ] in ( model , Effect.batch runEffects From 18947270338e8a269a993f1eff9120bf2fbfa837 Mon Sep 17 00:00:00 2001 From: wass3r <1301201+wass3r@users.noreply.github.com> Date: Mon, 5 Aug 2024 11:34:49 -0500 Subject: [PATCH 11/22] do not refresh repos on org page automatically --- src/elm/Pages/Org_.elm | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/elm/Pages/Org_.elm b/src/elm/Pages/Org_.elm index ca2ef1682..7696caf5c 100644 --- a/src/elm/Pages/Org_.elm +++ b/src/elm/Pages/Org_.elm @@ -168,16 +168,7 @@ update shared route msg model = -- REFRESH Tick options -> - ( model - , Effect.getOrgRepos - { baseUrl = shared.velaAPIBaseURL - , session = shared.session - , onResponse = GetOrgReposResponse - , org = route.params.org - , pageNumber = Dict.get "page" route.query |> Maybe.andThen String.toInt - , perPage = Dict.get "perPage" route.query |> Maybe.andThen String.toInt - } - ) + ( model, Effect.none ) From 308819ebc2c50f4f2fd8d4a9d84cc8b529118025 Mon Sep 17 00:00:00 2001 From: wass3r <1301201+wass3r@users.noreply.github.com> Date: Mon, 5 Aug 2024 11:38:58 -0500 Subject: [PATCH 12/22] not automatic refresh on secrets pages --- .../Pages/Dash/Secrets/Engine_/Org/Org_.elm | 24 +------------------ .../Dash/Secrets/Engine_/Repo/Org_/Repo_.elm | 24 +------------------ .../Secrets/Engine_/Shared/Org_/Team_.elm | 13 +--------- 3 files changed, 3 insertions(+), 58 deletions(-) diff --git a/src/elm/Pages/Dash/Secrets/Engine_/Org/Org_.elm b/src/elm/Pages/Dash/Secrets/Engine_/Org/Org_.elm index 12dc70499..d563a157c 100644 --- a/src/elm/Pages/Dash/Secrets/Engine_/Org/Org_.elm +++ b/src/elm/Pages/Dash/Secrets/Engine_/Org/Org_.elm @@ -216,29 +216,7 @@ update shared route msg model = -- REFRESH Tick options -> - ( model - , Effect.batch - [ Effect.getOrgSecrets - { baseUrl = shared.velaAPIBaseURL - , session = shared.session - , onResponse = GetOrgSecretsResponse - , pageNumber = Dict.get "page" route.query |> Maybe.andThen String.toInt - , perPage = Dict.get "perPage" route.query |> Maybe.andThen String.toInt - , engine = route.params.engine - , org = route.params.org - } - , Effect.getSharedSecrets - { baseUrl = shared.velaAPIBaseURL - , session = shared.session - , onResponse = GetSharedSecretsResponse - , pageNumber = Nothing - , perPage = Nothing - , engine = route.params.engine - , org = route.params.org - , team = "*" - } - ] - ) + ( model, Effect.none ) diff --git a/src/elm/Pages/Dash/Secrets/Engine_/Repo/Org_/Repo_.elm b/src/elm/Pages/Dash/Secrets/Engine_/Repo/Org_/Repo_.elm index bcc0d59e4..86831b172 100644 --- a/src/elm/Pages/Dash/Secrets/Engine_/Repo/Org_/Repo_.elm +++ b/src/elm/Pages/Dash/Secrets/Engine_/Repo/Org_/Repo_.elm @@ -204,29 +204,7 @@ update shared route msg model = -- REFRESH Tick options -> - ( model - , Effect.batch - [ Effect.getRepoSecrets - { baseUrl = shared.velaAPIBaseURL - , session = shared.session - , onResponse = GetRepoSecretsResponse - , pageNumber = Dict.get "page" route.query |> Maybe.andThen String.toInt - , perPage = Dict.get "perPage" route.query |> Maybe.andThen String.toInt - , engine = route.params.engine - , org = route.params.org - , repo = route.params.repo - } - , Effect.getOrgSecrets - { baseUrl = shared.velaAPIBaseURL - , session = shared.session - , onResponse = GetOrgSecretsResponse - , pageNumber = Nothing - , perPage = Nothing - , engine = route.params.engine - , org = route.params.org - } - ] - ) + ( model, Effect.none ) diff --git a/src/elm/Pages/Dash/Secrets/Engine_/Shared/Org_/Team_.elm b/src/elm/Pages/Dash/Secrets/Engine_/Shared/Org_/Team_.elm index b1b3cae1a..b20ad5c95 100644 --- a/src/elm/Pages/Dash/Secrets/Engine_/Shared/Org_/Team_.elm +++ b/src/elm/Pages/Dash/Secrets/Engine_/Shared/Org_/Team_.elm @@ -176,18 +176,7 @@ update shared route msg model = -- REFRESH Tick options -> - ( model - , Effect.getSharedSecrets - { baseUrl = shared.velaAPIBaseURL - , session = shared.session - , onResponse = GetSharedSecretsResponse - , pageNumber = Dict.get "page" route.query |> Maybe.andThen String.toInt - , perPage = Dict.get "perPage" route.query |> Maybe.andThen String.toInt - , engine = route.params.engine - , org = route.params.org - , team = route.params.team - } - ) + ( model, Effect.none ) From a36120dd4ffeef8556937a495c153341a2195795 Mon Sep 17 00:00:00 2001 From: wass3r <1301201+wass3r@users.noreply.github.com> Date: Mon, 5 Aug 2024 14:10:18 -0500 Subject: [PATCH 13/22] update global hooks from hooks page to make tab alerting work on errors; when this page was reduced to only produce one call to update hooks, it didn't update tab status because that's handled by the upper-level layout. this adopts same pattern as source repos --- src/elm/Effect.elm | 7 +- src/elm/Pages/Org_/Repo_/Hooks.elm | 152 +++++++++++++++-------------- src/elm/Shared.elm | 5 + src/elm/Shared/Msg.elm | 1 + 4 files changed, 90 insertions(+), 75 deletions(-) diff --git a/src/elm/Effect.elm b/src/elm/Effect.elm index 7b8817a13..b841ead73 100644 --- a/src/elm/Effect.elm +++ b/src/elm/Effect.elm @@ -9,7 +9,7 @@ module Effect exposing , sendCmd, sendMsg , pushRoute, replaceRoute, loadExternalUrl , map, toCmd - , addAlertError, addAlertSuccess, addDeployment, addFavorites, addOrgSecret, addRepoSchedule, addRepoSecret, addSharedSecret, alertsUpdate, approveBuild, cancelBuild, chownRepo, clearRedirect, deleteOrgSecret, deleteRepoSchedule, deleteRepoSecret, deleteSharedSecret, disableRepo, downloadFile, enableRepo, expandPipelineConfig, finishAuthentication, focusOn, getBuild, getBuildGraph, getBuildServiceLog, getBuildServices, getBuildStepLog, getBuildSteps, getCurrentUser, getCurrentUserShared, getDashboard, getOrgBuilds, getOrgRepos, getOrgSecret, getOrgSecrets, getPipelineConfig, getPipelineTemplates, getRepo, getRepoBuilds, getRepoBuildsShared, getRepoDeployments, getRepoHooks, getRepoHooksShared, getRepoSchedule, getRepoSchedules, getRepoSecret, getRepoSecrets, getSettings, getSharedSecret, getSharedSecrets, getWorkers, handleHttpError, logout, pushPath, redeliverHook, repairRepo, replacePath, replaceRouteRemoveTabHistorySkipDomFocus, restartBuild, setRedirect, setTheme, updateFavicon, updateFavorite, updateOrgSecret, updateRepo, updateRepoSchedule, updateRepoSecret, updateSettings, updateSharedSecret, updateSourceReposShared + , addAlertError, addAlertSuccess, addDeployment, addFavorites, addOrgSecret, addRepoSchedule, addRepoSecret, addSharedSecret, alertsUpdate, approveBuild, cancelBuild, chownRepo, clearRedirect, deleteOrgSecret, deleteRepoSchedule, deleteRepoSecret, deleteSharedSecret, disableRepo, downloadFile, enableRepo, expandPipelineConfig, finishAuthentication, focusOn, getBuild, getBuildGraph, getBuildServiceLog, getBuildServices, getBuildStepLog, getBuildSteps, getCurrentUser, getCurrentUserShared, getDashboard, getOrgBuilds, getOrgRepos, getOrgSecret, getOrgSecrets, getPipelineConfig, getPipelineTemplates, getRepo, getRepoBuilds, getRepoBuildsShared, getRepoDeployments, getRepoHooks, getRepoHooksShared, getRepoSchedule, getRepoSchedules, getRepoSecret, getRepoSecrets, getSettings, getSharedSecret, getSharedSecrets, getWorkers, handleHttpError, logout, pushPath, redeliverHook, repairRepo, replacePath, replaceRouteRemoveTabHistorySkipDomFocus, restartBuild, setRedirect, setTheme, updateFavicon, updateFavorite, updateOrgSecret, updateRepo, updateRepoHooksShared, updateRepoSchedule, updateRepoSecret, updateSettings, updateSharedSecret, updateSourceReposShared ) {-| @@ -626,6 +626,11 @@ getRepoHooksShared options = SendSharedMsg <| Shared.Msg.GetRepoHooks options +updateRepoHooksShared : { hooks : WebData (List Vela.Hook) } -> Effect msg +updateRepoHooksShared options = + SendSharedMsg <| Shared.Msg.UpdateRepoHooks options + + redeliverHook : { baseUrl : String , session : Auth.Session.Session diff --git a/src/elm/Pages/Org_/Repo_/Hooks.elm b/src/elm/Pages/Org_/Repo_/Hooks.elm index 5252f2805..0b426f0e4 100644 --- a/src/elm/Pages/Org_/Repo_/Hooks.elm +++ b/src/elm/Pages/Org_/Repo_/Hooks.elm @@ -157,91 +157,95 @@ type Msg -} update : Shared.Model -> Route { org : String, repo : String } -> Msg -> Model -> ( Model, Effect Msg ) update shared route msg model = - case msg of - -- HOOKS - GetRepoHooksResponse response -> - case response of - Ok ( meta, hooks ) -> - ( { model - | hooks = RemoteData.succeed hooks - , pager = Api.Pagination.get meta.headers - } - , Effect.none - ) - - Err error -> - ( { model | hooks = Errors.toFailure error } - , Effect.handleHttpError - { error = error - , shouldShowAlertFn = Errors.showAlertAlways - } - ) - - RedeliverRepoHook options -> - ( model - , Effect.redeliverHook - { baseUrl = shared.velaAPIBaseURL - , session = shared.session - , onResponse = RedeliverRepoHookResponse options - , org = route.params.org - , repo = route.params.repo - , hookNumber = String.fromInt <| options.hook.number - } - ) - - RedeliverRepoHookResponse options response -> - case response of - Ok ( _, result ) -> - ( model - , Effect.addAlertSuccess - { content = "Hook #" ++ (String.fromInt <| options.hook.number) ++ " redelivered successfully." - , addToastIfUnique = False - , link = Nothing + (\( m, e ) -> + ( m, Effect.batch [ e, Effect.updateRepoHooksShared { hooks = m.hooks } ] ) + ) + <| + case msg of + -- HOOKS + GetRepoHooksResponse response -> + case response of + Ok ( meta, hooks ) -> + ( { model + | hooks = RemoteData.succeed hooks + , pager = Api.Pagination.get meta.headers + } + , Effect.none + ) + + Err error -> + ( { model | hooks = Errors.toFailure error } + , Effect.handleHttpError + { error = error + , shouldShowAlertFn = Errors.showAlertAlways + } + ) + + RedeliverRepoHook options -> + ( model + , Effect.redeliverHook + { baseUrl = shared.velaAPIBaseURL + , session = shared.session + , onResponse = RedeliverRepoHookResponse options + , org = route.params.org + , repo = route.params.repo + , hookNumber = String.fromInt <| options.hook.number + } + ) + + RedeliverRepoHookResponse options response -> + case response of + Ok ( _, result ) -> + ( model + , Effect.addAlertSuccess + { content = "Hook #" ++ (String.fromInt <| options.hook.number) ++ " redelivered successfully." + , addToastIfUnique = False + , link = Nothing + } + ) + + Err error -> + ( model + , Effect.handleHttpError + { error = error + , shouldShowAlertFn = Errors.showAlertAlways + } + ) + + GotoPage pageNumber -> + ( model + , Effect.batch + [ Effect.replaceRoute + { path = route.path + , query = + Dict.update "page" (\_ -> Just <| String.fromInt pageNumber) route.query + , hash = route.hash } - ) - - Err error -> - ( model - , Effect.handleHttpError - { error = error - , shouldShowAlertFn = Errors.showAlertAlways + , Effect.getRepoHooks + { baseUrl = shared.velaAPIBaseURL + , session = shared.session + , onResponse = GetRepoHooksResponse + , pageNumber = Just pageNumber + , perPage = Dict.get "perPage" route.query |> Maybe.andThen String.toInt + , org = route.params.org + , repo = route.params.repo } - ) + ] + ) - GotoPage pageNumber -> - ( model - , Effect.batch - [ Effect.replaceRoute - { path = route.path - , query = - Dict.update "page" (\_ -> Just <| String.fromInt pageNumber) route.query - , hash = route.hash - } + -- REFRESH + Tick options -> + ( model , Effect.getRepoHooks { baseUrl = shared.velaAPIBaseURL , session = shared.session , onResponse = GetRepoHooksResponse - , pageNumber = Just pageNumber + , pageNumber = Dict.get "page" route.query |> Maybe.andThen String.toInt , perPage = Dict.get "perPage" route.query |> Maybe.andThen String.toInt , org = route.params.org , repo = route.params.repo } - ] - ) - - -- REFRESH - Tick options -> - ( model - , Effect.getRepoHooks - { baseUrl = shared.velaAPIBaseURL - , session = shared.session - , onResponse = GetRepoHooksResponse - , pageNumber = Dict.get "page" route.query |> Maybe.andThen String.toInt - , perPage = Dict.get "perPage" route.query |> Maybe.andThen String.toInt - , org = route.params.org - , repo = route.params.repo - } - ) + ) diff --git a/src/elm/Shared.elm b/src/elm/Shared.elm index ef12ac393..6d5475628 100644 --- a/src/elm/Shared.elm +++ b/src/elm/Shared.elm @@ -612,6 +612,11 @@ update route msg model = } ) + Shared.Msg.UpdateRepoHooks options -> + ( { model | hooks = options.hooks } + , Effect.none + ) + -- THEME Shared.Msg.SetTheme options -> if options.theme == model.theme then diff --git a/src/elm/Shared/Msg.elm b/src/elm/Shared/Msg.elm index c8a7c29cb..1392ba7ce 100644 --- a/src/elm/Shared/Msg.elm +++ b/src/elm/Shared/Msg.elm @@ -59,6 +59,7 @@ type Msg -- HOOKS | GetRepoHooks { org : String, repo : String, pageNumber : Maybe Int, perPage : Maybe Int, maybeEvent : Maybe String } | GetRepoHooksResponse (Result (Http.Detailed.Error String) ( Http.Metadata, List Vela.Hook )) + | UpdateRepoHooks { hooks : WebData (List Vela.Hook) } -- THEME | SetTheme { theme : Theme.Theme } -- ALERTS From 89ebb1a5aea0079223bb5de18793a70f81ddc224 Mon Sep 17 00:00:00 2001 From: wass3r <1301201+wass3r@users.noreply.github.com> Date: Mon, 5 Aug 2024 14:47:21 -0500 Subject: [PATCH 14/22] fix test this step won't update if it's not running --- cypress/fixtures/steps_5.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cypress/fixtures/steps_5.json b/cypress/fixtures/steps_5.json index 34ff91cf5..c64e5d5cf 100644 --- a/cypress/fixtures/steps_5.json +++ b/cypress/fixtures/steps_5.json @@ -23,12 +23,12 @@ "number": 2, "name": "build", "stage": "", - "status": "success", + "status": "running", "error": "", "exit_code": 2, "created": 1572029883, "started": 1572029928, - "finished": 1572029935, + "finished": 0, "host": "", "runtime": "docker", "distribution": "linux" From 815b04b863a1d9f374ca26380b701a2da63522bf Mon Sep 17 00:00:00 2001 From: wass3r <1301201+wass3r@users.noreply.github.com> Date: Tue, 6 Aug 2024 13:42:43 -0500 Subject: [PATCH 15/22] add note for step log refreshing --- src/elm/Pages/Org_/Repo_/Build_.elm | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/elm/Pages/Org_/Repo_/Build_.elm b/src/elm/Pages/Org_/Repo_/Build_.elm index 1a2e26b33..7ad635107 100644 --- a/src/elm/Pages/Org_/Repo_/Build_.elm +++ b/src/elm/Pages/Org_/Repo_/Build_.elm @@ -322,6 +322,12 @@ update shared route msg model = ( { model | steps = RemoteData.succeed <| List.sortBy .number steps } , steps |> List.filter (\step -> List.member step.number model.viewing) + -- note: it's possible that there are log updates in flight + -- even after the step has a status of finished, especially + -- for large logs. we get the most recent version of logs + -- on page load or when a step log is expanded, so potentially + -- seeing incomplete logs here is only a concern when someone + -- is following the logs live. |> List.filter (\step -> step.finished == 0) |> List.map (\step -> From 8f476521ab36d7087a8475f65ea3ed83fe09c4aa Mon Sep 17 00:00:00 2001 From: wass3r <1301201+wass3r@users.noreply.github.com> Date: Tue, 6 Aug 2024 13:44:10 -0500 Subject: [PATCH 16/22] handle 401s on non-token-refresh calls will redirect to last page you were on. some opportunity here to improve messaging on login screen, or maybe sticky toast signaling reason you are not on the page you were on before. --- src/elm/Shared.elm | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/src/elm/Shared.elm b/src/elm/Shared.elm index 6d5475628..fc06b665e 100644 --- a/src/elm/Shared.elm +++ b/src/elm/Shared.elm @@ -667,21 +667,24 @@ update route msg model = let ( shared, redirect ) = case options.error of - -- todo: maybe we pass in a status code we want to ignore - -- so secrets can skip this alert for 401s - -- - -- Http.Detailed.BadStatus meta _ -> - -- case meta.statusCode of - -- -- todo: FIX THIS! secrets can easily return a 401 for normal reasons - -- 401 -> - -- ( { model - -- | session = Auth.Session.Unauthenticated - -- , velaRedirect = "/" - -- } - -- , Effect.replacePath <| Route.Path.Account_Login - -- ) - -- _ -> - -- ( model, Effect.none ) + -- only handles token expiration on 401s + -- TODO: handle 401s for access restriction reasons + Http.Detailed.BadStatus meta body -> + let + isTokenExpired = + String.contains "token is expired" body + in + if meta.statusCode == 401 && isTokenExpired then + ( { model + | session = Auth.Session.Unauthenticated + , velaRedirect = Route.Path.toString route.path + } + , Effect.replacePath <| Route.Path.Account_Login + ) + + else + ( model, Effect.none ) + _ -> ( model, Effect.none ) in From d1c01a0084fd8f71b0e18316f62c83b3eca40b6c Mon Sep 17 00:00:00 2001 From: wass3r <1301201+wass3r@users.noreply.github.com> Date: Tue, 6 Aug 2024 13:50:22 -0500 Subject: [PATCH 17/22] rename and add note --- src/elm/Layouts/Default/Repo.elm | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/elm/Layouts/Default/Repo.elm b/src/elm/Layouts/Default/Repo.elm index 8a7c45850..3727ff4ad 100644 --- a/src/elm/Layouts/Default/Repo.elm +++ b/src/elm/Layouts/Default/Repo.elm @@ -149,11 +149,13 @@ update props route msg model = -- REFRESH Tick options -> let - shouldRefreshHooks = + -- the hooks page has its own refresh call for hooks; + -- this is to prevent double calls + isNotOnHooksPage = route.path /= Route.Path.Org__Repo__Hooks { org = props.org, repo = props.repo } runEffect = - if shouldRefreshHooks then + if isNotOnHooksPage then Effect.getRepoHooksShared { pageNumber = Nothing , perPage = Nothing From 30ca1d25652a78ead2f7cba97a08b7ac5b134e83 Mon Sep 17 00:00:00 2001 From: wass3r <1301201+wass3r@users.noreply.github.com> Date: Tue, 6 Aug 2024 13:52:41 -0500 Subject: [PATCH 18/22] rename var --- src/elm/Pages/Org_/Repo_/Build_/Graph.elm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/elm/Pages/Org_/Repo_/Build_/Graph.elm b/src/elm/Pages/Org_/Repo_/Build_/Graph.elm index 88338aec7..0000438be 100644 --- a/src/elm/Pages/Org_/Repo_/Build_/Graph.elm +++ b/src/elm/Pages/Org_/Repo_/Build_/Graph.elm @@ -239,7 +239,7 @@ update shared route msg model = Refresh options -> let - shouldRefresh = + isBuildRunning = case shared.builds of RemoteData.Success builds -> List.Extra.find (\b -> b.number == Maybe.withDefault 0 (String.toInt route.params.build)) builds @@ -250,7 +250,7 @@ update shared route msg model = False runEffect = - if shouldRefresh then + if isBuildRunning then Effect.getBuildGraph { baseUrl = shared.velaAPIBaseURL , session = shared.session From 15b46f98d83b2e4e5030604f8e107848de37783f Mon Sep 17 00:00:00 2001 From: wass3r <1301201+wass3r@users.noreply.github.com> Date: Tue, 6 Aug 2024 13:55:35 -0500 Subject: [PATCH 19/22] add note --- src/elm/Pages/Org_/Repo_/Hooks.elm | 1 + 1 file changed, 1 insertion(+) diff --git a/src/elm/Pages/Org_/Repo_/Hooks.elm b/src/elm/Pages/Org_/Repo_/Hooks.elm index 0b426f0e4..cf95e6a08 100644 --- a/src/elm/Pages/Org_/Repo_/Hooks.elm +++ b/src/elm/Pages/Org_/Repo_/Hooks.elm @@ -157,6 +157,7 @@ type Msg -} update : Shared.Model -> Route { org : String, repo : String } -> Msg -> Model -> ( Model, Effect Msg ) update shared route msg model = + -- persist any hooks updates to the shared model (\( m, e ) -> ( m, Effect.batch [ e, Effect.updateRepoHooksShared { hooks = m.hooks } ] ) ) From 883b4843ed5140c13b9eee63a686b6c16d6cf5f6 Mon Sep 17 00:00:00 2001 From: wass3r <1301201+wass3r@users.noreply.github.com> Date: Tue, 6 Aug 2024 22:47:35 -0500 Subject: [PATCH 20/22] add step logic to services as well --- src/elm/Pages/Org_/Repo_/Build_/Services.elm | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/elm/Pages/Org_/Repo_/Build_/Services.elm b/src/elm/Pages/Org_/Repo_/Build_/Services.elm index 393d5df59..cde941f06 100644 --- a/src/elm/Pages/Org_/Repo_/Build_/Services.elm +++ b/src/elm/Pages/Org_/Repo_/Build_/Services.elm @@ -313,6 +313,13 @@ update shared route msg model = ( { model | services = RemoteData.succeed services } , services |> List.filter (\service -> List.member service.number model.viewing) + -- note: it's possible that there are log updates in flight + -- even after the service has a status of finished, especially + -- for large logs. we get the most recent version of logs + -- on page load or when a service log is expanded, so potentially + -- seeing incomplete logs here is only a concern when someone + -- is following the logs live. + |> List.filter (\service -> service.finished == 0) |> List.map (\service -> Effect.getBuildServiceLog From 5c7cf2d3f81887dbab4fedd0ef43b4d628f1a85b Mon Sep 17 00:00:00 2001 From: wass3r <1301201+wass3r@users.noreply.github.com> Date: Tue, 6 Aug 2024 23:49:02 -0500 Subject: [PATCH 21/22] fix test --- cypress/fixtures/services_5.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cypress/fixtures/services_5.json b/cypress/fixtures/services_5.json index 1722a510b..5c34d17fc 100644 --- a/cypress/fixtures/services_5.json +++ b/cypress/fixtures/services_5.json @@ -23,12 +23,12 @@ "number": 2, "name": "service-b", "stage": "", - "status": "success", + "status": "running", "error": "", "exit_code": 2, "created": 1572029883, "started": 1572029928, - "finished": 1572029935, + "finished": 0, "host": "", "runtime": "docker", "distribution": "linux" From 6fe56ddeadd80f7cbbbf17db5751d892c0b61f69 Mon Sep 17 00:00:00 2001 From: dave vader <48764154+plyr4@users.noreply.github.com> Date: Wed, 7 Aug 2024 13:31:28 -0500 Subject: [PATCH 22/22] fix: stop double log fetch on steps/services (#813) --- src/elm/Pages/Org_/Repo_/Build_.elm | 111 +++++++++++++----- src/elm/Pages/Org_/Repo_/Build_/Services.elm | 112 ++++++++++++++----- src/elm/Utils/Helpers.elm | 15 ++- 3 files changed, 182 insertions(+), 56 deletions(-) diff --git a/src/elm/Pages/Org_/Repo_/Build_.elm b/src/elm/Pages/Org_/Repo_/Build_.elm index 7ad635107..91b751220 100644 --- a/src/elm/Pages/Org_/Repo_/Build_.elm +++ b/src/elm/Pages/Org_/Repo_/Build_.elm @@ -235,7 +235,7 @@ type Msg | GetBuildStepLogResponse { step : Vela.Step, applyDomFocus : Bool, previousFocus : Maybe Focus.Focus } (Result (Http.Detailed.Error String) ( Http.Metadata, Vela.Log )) | GetBuildStepLogRefreshResponse { step : Vela.Step } (Result (Http.Detailed.Error String) ( Http.Metadata, Vela.Log )) | ClickStep { step : Vela.Step } - | ExpandStep { step : Vela.Step, applyDomFocus : Bool, previousFocus : Maybe Focus.Focus } + | ExpandStep { step : Vela.Step, applyDomFocus : Bool, previousFocus : Maybe Focus.Focus, triggeredFromClick : Bool } | CollapseStep { step : Vela.Step } | ExpandAll | CollapseAll @@ -265,7 +265,15 @@ update shared route msg model = } , RemoteData.withDefault [] model.steps |> List.filter (\s -> Maybe.withDefault -1 focus.group == s.number) - |> List.map (\s -> ExpandStep { step = s, applyDomFocus = True, previousFocus = Just model.focus }) + |> List.map + (\s -> + ExpandStep + { step = s + , applyDomFocus = True + , previousFocus = Just model.focus + , triggeredFromClick = False + } + ) |> List.map Effect.sendMsg |> Effect.batch ) @@ -302,6 +310,7 @@ update shared route msg model = { step = step , applyDomFocus = options.applyDomFocus , previousFocus = Nothing + , triggeredFromClick = False } |> Effect.sendMsg ) @@ -439,7 +448,12 @@ update shared route msg model = else Effect.batch - [ ExpandStep { step = options.step, applyDomFocus = False, previousFocus = Nothing } + [ ExpandStep + { step = options.step + , applyDomFocus = False + , previousFocus = Nothing + , triggeredFromClick = True + } |> Effect.sendMsg , case model.focus.a of Nothing -> @@ -459,25 +473,57 @@ update shared route msg model = ) ExpandStep options -> - ( { model - | viewing = List.Extra.unique <| options.step.number :: model.viewing - } - , Effect.batch - [ Effect.getBuildStepLog - { baseUrl = shared.velaAPIBaseURL - , session = shared.session - , onResponse = - GetBuildStepLogResponse - { step = options.step - , applyDomFocus = options.applyDomFocus - , previousFocus = options.previousFocus - } - , org = route.params.org - , repo = route.params.repo - , build = route.params.build - , stepNumber = String.fromInt options.step.number - } - , if options.applyDomFocus then + let + isFromHashChanged = + options.previousFocus /= Nothing + + didFocusChange = + case options.previousFocus of + Just f -> + f.group /= model.focus.group + + Nothing -> + False + + -- hash will change when no line is selected and the selected group changes + -- this means expansion msg will double up on fetching logs unless instructed not to + willFocusChange = + case ( model.focus.group, model.focus.a, model.focus.b ) of + ( Just g, Nothing, _ ) -> + g /= options.step.number + + _ -> + False + + isLogLoaded = + Dict.get options.step.id model.logs + |> Maybe.withDefault RemoteData.Loading + |> Util.isLoaded + + -- fetch logs when expansion msg meets the criteria: + -- triggered by a click that will change the hash + -- the focus changes and the logs are not loaded + fetchLogs = + not (options.triggeredFromClick && willFocusChange) + && ((didFocusChange && not isLogLoaded) || not isFromHashChanged) + + getLogEffect = + Effect.getBuildStepLog + { baseUrl = shared.velaAPIBaseURL + , session = shared.session + , onResponse = + GetBuildStepLogResponse + { step = options.step + , applyDomFocus = options.applyDomFocus + , previousFocus = options.previousFocus + } + , org = route.params.org + , repo = route.params.repo + , build = route.params.build + , stepNumber = String.fromInt options.step.number + } + + applyDomFocusEffect = case ( model.focus.group, model.focus.a, model.focus.b ) of ( Just g, Nothing, Nothing ) -> FocusOn @@ -493,9 +539,23 @@ update shared route msg model = _ -> Effect.none - else - Effect.none - ] + runEffects = + [ if fetchLogs then + getLogEffect + + else + Effect.none + , if options.applyDomFocus then + applyDomFocusEffect + + else + Effect.none + ] + in + ( { model + | viewing = List.Extra.unique <| options.step.number :: model.viewing + } + , Effect.batch runEffects ) CollapseStep options -> @@ -525,6 +585,7 @@ update shared route msg model = { step = step , applyDomFocus = False , previousFocus = Nothing + , triggeredFromClick = False } ) |> List.map Effect.sendMsg diff --git a/src/elm/Pages/Org_/Repo_/Build_/Services.elm b/src/elm/Pages/Org_/Repo_/Build_/Services.elm index cde941f06..67249ab72 100644 --- a/src/elm/Pages/Org_/Repo_/Build_/Services.elm +++ b/src/elm/Pages/Org_/Repo_/Build_/Services.elm @@ -227,7 +227,7 @@ type Msg | GetBuildServiceLogResponse { service : Vela.Service, applyDomFocus : Bool, previousFocus : Maybe Focus.Focus } (Result (Http.Detailed.Error String) ( Http.Metadata, Vela.Log )) | GetBuildServiceLogRefreshResponse { service : Vela.Service } (Result (Http.Detailed.Error String) ( Http.Metadata, Vela.Log )) | ClickService { service : Vela.Service } - | ExpandService { service : Vela.Service, applyDomFocus : Bool, previousFocus : Maybe Focus.Focus } + | ExpandService { service : Vela.Service, applyDomFocus : Bool, previousFocus : Maybe Focus.Focus, triggeredFromClick : Bool } | CollapseService { service : Vela.Service } | ExpandAll | CollapseAll @@ -257,7 +257,15 @@ update shared route msg model = } , RemoteData.withDefault [] model.services |> List.filter (\s -> Maybe.withDefault -1 focus.group == s.number) - |> List.map (\s -> ExpandService { service = s, applyDomFocus = True, previousFocus = Just model.focus }) + |> List.map + (\s -> + ExpandService + { service = s + , applyDomFocus = True + , previousFocus = Just model.focus + , triggeredFromClick = False + } + ) |> List.map Effect.sendMsg |> Effect.batch ) @@ -293,6 +301,7 @@ update shared route msg model = { service = service , applyDomFocus = True , previousFocus = Nothing + , triggeredFromClick = False } |> Effect.sendMsg ) @@ -430,7 +439,12 @@ update shared route msg model = else Effect.batch - [ ExpandService { service = options.service, applyDomFocus = False, previousFocus = Nothing } + [ ExpandService + { service = options.service + , applyDomFocus = False + , previousFocus = Nothing + , triggeredFromClick = True + } |> Effect.sendMsg , case model.focus.a of Nothing -> @@ -450,25 +464,57 @@ update shared route msg model = ) ExpandService options -> - ( { model - | viewing = List.Extra.unique <| options.service.number :: model.viewing - } - , Effect.batch - [ Effect.getBuildServiceLog - { baseUrl = shared.velaAPIBaseURL - , session = shared.session - , onResponse = - GetBuildServiceLogResponse - { service = options.service - , applyDomFocus = options.applyDomFocus - , previousFocus = options.previousFocus - } - , org = route.params.org - , repo = route.params.repo - , build = route.params.build - , serviceNumber = String.fromInt options.service.number - } - , if options.applyDomFocus then + let + isFromHashChanged = + options.previousFocus /= Nothing + + didFocusChange = + case options.previousFocus of + Just f -> + f.group /= model.focus.group + + Nothing -> + False + + -- hash will change when no line is selected and the selected group changes + -- this means the expansion msg will double up on fetching logs unless instructed not to + willFocusChange = + case ( model.focus.group, model.focus.a, model.focus.b ) of + ( Just g, Nothing, _ ) -> + g /= options.service.number + + _ -> + False + + isLogLoaded = + Dict.get options.service.id model.logs + |> Maybe.withDefault RemoteData.Loading + |> Util.isLoaded + + -- fetch logs when expansion msg meets the criteria: + -- triggered by a click that will change the hash + -- the focus changes and the logs are not loaded + fetchLogs = + not (options.triggeredFromClick && willFocusChange) + && ((didFocusChange && not isLogLoaded) || not isFromHashChanged) + + getLogEffect = + Effect.getBuildServiceLog + { baseUrl = shared.velaAPIBaseURL + , session = shared.session + , onResponse = + GetBuildServiceLogResponse + { service = options.service + , applyDomFocus = options.applyDomFocus + , previousFocus = options.previousFocus + } + , org = route.params.org + , repo = route.params.repo + , build = route.params.build + , serviceNumber = String.fromInt options.service.number + } + + applyDomFocusEffect = case ( model.focus.group, model.focus.a, model.focus.b ) of ( Just g, Nothing, Nothing ) -> FocusOn @@ -484,9 +530,24 @@ update shared route msg model = _ -> Effect.none - else - Effect.none - ] + runEffects = + [ if fetchLogs then + getLogEffect + + else + Effect.none + , if options.applyDomFocus then + applyDomFocusEffect + + else + Effect.none + ] + in + ( { model + | viewing = List.Extra.unique <| options.service.number :: model.viewing + } + , Effect.batch + runEffects ) CollapseService options -> @@ -516,6 +577,7 @@ update shared route msg model = { service = service , applyDomFocus = False , previousFocus = Nothing + , triggeredFromClick = False } ) |> List.map Effect.sendMsg diff --git a/src/elm/Utils/Helpers.elm b/src/elm/Utils/Helpers.elm index 470bf98ff..6b3b8f532 100644 --- a/src/elm/Utils/Helpers.elm +++ b/src/elm/Utils/Helpers.elm @@ -26,7 +26,7 @@ module Utils.Helpers exposing , humanReadableDateTimeFormatter , humanReadableDateTimeWithDefault , humanReadableDateWithDefault - , isLoading + , isLoaded , isSuccess , mergeListsById , noBlanks @@ -375,17 +375,20 @@ fiveSecondsMillis = oneSecondMillis * 5 -{-| isLoading : takes WebData and returns true if it is in a Loading state. +{-| isLoaded : takes WebData and returns true if it is in a 'loaded' state, meaning its anything but NotAsked or Loading. -} -isLoading : WebData a -> Bool -isLoading status = +isLoaded : WebData a -> Bool +isLoaded status = case status of RemoteData.Loading -> - True + False - _ -> + RemoteData.NotAsked -> False + _ -> + True + {-| isSuccess : takes WebData and returns true if it is in a Success state. -}