From b83bba0661c7f2d4f5efc995c303cfd38fc3470d Mon Sep 17 00:00:00 2001 From: david may <1301201+wass3r@users.noreply.github.com> Date: Wed, 7 Aug 2024 19:37:35 +0000 Subject: [PATCH] fix: reduce excessive API calls (#810) Co-authored-by: davidvader Co-authored-by: dave vader <48764154+plyr4@users.noreply.github.com> --- cypress/fixtures/services_5.json | 4 +- cypress/fixtures/steps_5.json | 4 +- src/elm/Effect.elm | 7 +- src/elm/Layouts/Default/Build.elm | 52 ++++-- src/elm/Layouts/Default/Repo.elm | 37 +++-- .../Pages/Dash/Secrets/Engine_/Org/Org_.elm | 24 +-- .../Dash/Secrets/Engine_/Repo/Org_/Repo_.elm | 24 +-- .../Secrets/Engine_/Shared/Org_/Team_.elm | 13 +- src/elm/Pages/Home_.elm | 2 +- src/elm/Pages/Org_.elm | 11 +- src/elm/Pages/Org_/Repo_/Build_.elm | 154 +++++++++++++---- src/elm/Pages/Org_/Repo_/Build_/Graph.elm | 49 ++++-- src/elm/Pages/Org_/Repo_/Build_/Services.elm | 155 ++++++++++++++---- src/elm/Pages/Org_/Repo_/Hooks.elm | 153 ++++++++--------- src/elm/Shared.elm | 38 +++-- src/elm/Shared/Msg.elm | 1 + src/elm/Utils/Helpers.elm | 15 +- 17 files changed, 459 insertions(+), 284 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" 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" 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/Layouts/Default/Build.elm b/src/elm/Layouts/Default/Build.elm index e4adaf2d0..9c1aef9bd 100644 --- a/src/elm/Layouts/Default/Build.elm +++ b/src/elm/Layouts/Default/Build.elm @@ -328,24 +328,42 @@ update props shared route msg model = -- REFRESH Tick options -> + let + isBuildRunning = + case model.build of + RemoteData.Success build -> + build.finished == 0 + + _ -> + True + + getRepoBuildsEffect = + Effect.getRepoBuildsShared + { pageNumber = Nothing + , perPage = Nothing + , maybeEvent = Nothing + , org = props.org + , repo = props.repo + } + + runEffects = + if isBuildRunning then + [ getRepoBuildsEffect + , Effect.getBuild + { baseUrl = shared.velaAPIBaseURL + , session = shared.session + , onResponse = GetBuildResponse + , org = props.org + , repo = props.repo + , build = props.build + } + ] + + else + [ getRepoBuildsEffect ] + 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..3727ff4ad 100644 --- a/src/elm/Layouts/Default/Repo.elm +++ b/src/elm/Layouts/Default/Repo.elm @@ -148,24 +148,27 @@ update props route msg model = -- REFRESH Tick options -> + let + -- 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 isNotOnHooksPage then + Effect.getRepoHooksShared + { pageNumber = Nothing + , perPage = Nothing + , maybeEvent = Nothing + , org = props.org + , repo = props.repo + } + + else + Effect.none + in ( model - , Effect.batch - [ Effect.getCurrentUserShared {} - , Effect.getRepoBuildsShared - { pageNumber = Nothing - , perPage = Nothing - , maybeEvent = Nothing - , org = props.org - , repo = props.repo - } - , Effect.getRepoHooksShared - { pageNumber = Nothing - , perPage = Nothing - , maybeEvent = Nothing - , org = props.org - , repo = props.repo - } - ] + , runEffect ) 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 ) 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_.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 ) diff --git a/src/elm/Pages/Org_/Repo_/Build_.elm b/src/elm/Pages/Org_/Repo_/Build_.elm index bd9d63a41..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 ) @@ -322,6 +331,13 @@ 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 -> Effect.getBuildStepLog @@ -432,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 -> @@ -452,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 @@ -486,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 -> @@ -518,6 +585,7 @@ update shared route msg model = { step = step , applyDomFocus = False , previousFocus = Nothing + , triggeredFromClick = False } ) |> List.map Effect.sendMsg @@ -546,17 +614,33 @@ update shared route msg model = -- REFRESH Tick options -> + let + isAnyStepRunning = + case model.steps of + RemoteData.Success steps -> + List.any (\s -> s.finished == 0) steps + + _ -> + False + + runEffect = + if isAnyStepRunning 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..0000438be 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 + isBuildRunning = + 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 isBuildRunning 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 @@ -337,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 } + ) @@ -357,6 +383,7 @@ subscriptions model = -- on visiblity changed, same as shared , Browser.Events.onVisibilityChange (\visibility -> VisibilityChanged { visibility = visibility }) + , Interval.tickEveryFiveSeconds Tick , Interval.tickEveryOneSecond Tick ] diff --git a/src/elm/Pages/Org_/Repo_/Build_/Services.elm b/src/elm/Pages/Org_/Repo_/Build_/Services.elm index a483bb64b..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 ) @@ -313,6 +322,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 @@ -423,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 -> @@ -443,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 @@ -477,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 -> @@ -509,6 +577,7 @@ update shared route msg model = { service = service , applyDomFocus = False , previousFocus = Nothing + , triggeredFromClick = False } ) |> List.map Effect.sendMsg @@ -537,17 +606,33 @@ update shared route msg model = -- REFRESH Tick options -> + let + isAnyServiceRunning = + case model.services of + RemoteData.Success services -> + List.any (\s -> s.status == Vela.Running) services + + _ -> + False + + runEffect = + if isAnyServiceRunning 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 ) diff --git a/src/elm/Pages/Org_/Repo_/Hooks.elm b/src/elm/Pages/Org_/Repo_/Hooks.elm index 5252f2805..cf95e6a08 100644 --- a/src/elm/Pages/Org_/Repo_/Hooks.elm +++ b/src/elm/Pages/Org_/Repo_/Hooks.elm @@ -157,91 +157,96 @@ 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 + -- persist any hooks updates to the shared model + (\( 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..fc06b665e 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 @@ -662,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 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 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. -}