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. -}