Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: render pipeline warnings as line annotations #831

Merged
merged 9 commits into from
Jan 10, 2025
Merged

feat: render pipeline warnings as line annotations #831

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
fd1c274
feat: render pipeline warnings as line annotations
plyr4 Jan 3, 2025
c8071a5
chore: cleanup
plyr4 Jan 3, 2025
156755b
fix: address feedback
plyr4 Jan 7, 2025
41c796c
feat: cypress tests
plyr4 Jan 7, 2025
e6482a5
fix: comment spacing elm-format hack
plyr4 Jan 7, 2025
ac01968
fix: scss organization
plyr4 Jan 7, 2025
86ab801
fix: disabled link constrast on light-theme
plyr4 Jan 7, 2025
127e47f
Merge branch 'main' into feat/pipeline-warnings
plyr4 Jan 10, 2025
d10bd56
Merge branch 'main' into feat/pipeline-warnings
wass3rw3rk Jan 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/elm/Components/Logs.elm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ viewLine pushUrlHashMsg resourceType resourceNumber shift focus logLine lineNumb
]
[ span [] [ text <| String.fromInt lineNumber ] ]
]
, td [ class "break-text", class "overflow-auto" ]
, td [ class "break-text", class "overflow-auto", class "line-content" ]
[ code [ Util.testAttribute <| String.join "-" [ "log", "data", resourceNumber, String.fromInt lineNumber ] ]
[ logLine.view
]
Expand Down
17 changes: 16 additions & 1 deletion src/elm/Components/Svgs.elm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ SPDX-License-Identifier: Apache-2.0


module Components.Svgs exposing
( buildStatusAnimation
( annotationCircle
, buildStatusAnimation
, buildStatusToIcon
, buildVizLegendEdge
, buildVizLegendNode
Expand Down Expand Up @@ -792,3 +793,17 @@ buildVizLegendEdge attrs =
)
[]
]


{-| annotationCircle : produces svg icon for line annotation bubble.
-}
annotationCircle : String -> Html msg
annotationCircle cls =
svg
[ class "-icon"
, class cls
, viewBox "0 0 48 48"
, width "22"
, height "22"
]
[ Svg.circle [ cx "24", cy "24", r "8" ] [] ]
219 changes: 165 additions & 54 deletions src/elm/Pages/Org_/Repo_/Build_/Pipeline.elm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,12 @@ import Ansi.Log
import Array
import Auth
import Components.Loading
import Components.Svgs
import Dict exposing (Dict)
import Effect exposing (Effect)
import FeatherIcons
import Html exposing (Html, a, button, code, details, div, small, span, strong, summary, table, td, text, tr)
import Html.Attributes exposing (attribute, class, href, id, target)
import Html.Attributes exposing (attribute, class, disabled, href, id, title)
import Html.Events exposing (onClick)
import Http
import Http.Detailed
Expand Down Expand Up @@ -220,25 +221,28 @@ update shared route msg model =
sideEffect =
case model.build of
RemoteData.Success build ->
if expand then
Effect.expandPipelineConfig
{ baseUrl = shared.velaAPIBaseURL
, session = shared.session
, onResponse = GetExpandBuildPipelineConfigResponse { applyDomFocus = False }
, org = route.params.org
, repo = route.params.repo
, ref = build.commit
}

else
Effect.getPipelineConfig
Effect.batch
[ Effect.getPipelineConfig
{ baseUrl = shared.velaAPIBaseURL
, session = shared.session
, onResponse = GetBuildPipelineConfigResponse { applyDomFocus = False }
, org = route.params.org
, repo = route.params.repo
, ref = build.commit
}
, if expand then
Effect.expandPipelineConfig
{ baseUrl = shared.velaAPIBaseURL
, session = shared.session
, onResponse = GetExpandBuildPipelineConfigResponse { applyDomFocus = False }
, org = route.params.org
, repo = route.params.repo
, ref = build.commit
}

else
Effect.none
]

_ ->
Effect.getBuild
Expand Down Expand Up @@ -314,31 +318,16 @@ update shared route msg model =
GetBuildResponse options response ->
case response of
Ok ( _, build ) ->
let
getPipelineConfigEffect =
if model.expand then
Effect.expandPipelineConfig
{ baseUrl = shared.velaAPIBaseURL
, session = shared.session
, onResponse = GetExpandBuildPipelineConfigResponse { applyDomFocus = options.applyDomFocus }
, org = route.params.org
, repo = route.params.repo
, ref = build.commit
}

else
Effect.getPipelineConfig
{ baseUrl = shared.velaAPIBaseURL
, session = shared.session
, onResponse = GetBuildPipelineConfigResponse { applyDomFocus = options.applyDomFocus }
, org = route.params.org
, repo = route.params.repo
, ref = build.commit
}
in
( { model | build = RemoteData.Success build }
, Effect.batch
[ getPipelineConfigEffect
[ Effect.getPipelineConfig
{ baseUrl = shared.velaAPIBaseURL
, session = shared.session
, onResponse = GetBuildPipelineConfigResponse { applyDomFocus = options.applyDomFocus }
, org = route.params.org
, repo = route.params.repo
, ref = build.commit
}
, Effect.getPipelineTemplates
{ baseUrl = shared.velaAPIBaseURL
, session = shared.session
Expand Down Expand Up @@ -370,12 +359,26 @@ update shared route msg model =
}
, expanding = False
}
, if Focus.canTarget model.focus && options.applyDomFocus then
FocusOn { target = Focus.toDomTarget model.focus }
|> Effect.sendMsg
, Effect.batch
[ if model.expand then
Effect.expandPipelineConfig
{ baseUrl = shared.velaAPIBaseURL
, session = shared.session
, onResponse = GetExpandBuildPipelineConfigResponse { applyDomFocus = options.applyDomFocus }
, org = route.params.org
, repo = route.params.repo
, ref = pipeline.commit
}

else
Effect.none
else
Effect.none
, if Focus.canTarget model.focus && options.applyDomFocus then
FocusOn { target = Focus.toDomTarget model.focus }
|> Effect.sendMsg

else
Effect.none
]
)

Err error ->
Expand All @@ -389,11 +392,16 @@ update shared route msg model =
GetExpandBuildPipelineConfigResponse options response ->
case response of
Ok ( _, expandedPipeline ) ->
let
p =
RemoteData.withDefault Vela.defaultPipelineConfig model.pipeline
in
( { model
| pipeline =
RemoteData.Success
{ rawData = ""
, decodedData = expandedPipeline
RemoteData.succeed
{ p
| rawData = ""
, decodedData = expandedPipeline
}
, expanding = False
}
Expand Down Expand Up @@ -486,7 +494,7 @@ view shared route model =

else
div [ class "icon" ] [ FeatherIcons.circle |> FeatherIcons.withSize 20 |> FeatherIcons.toHtml [] ]
, small [ class "tip" ] [ text "note: yaml fields will be sorted alphabetically when the pipeline is expanded." ]
, small [] [ text "note: yaml fields will be sorted alphabetically when the pipeline is expanded." ]
]

_ ->
Expand Down Expand Up @@ -522,7 +530,39 @@ view shared route model =
in
{ title = "Pipeline"
, body =
[ div [ class "pipeline" ]
[ case model.pipeline of
RemoteData.Success p ->
if List.length p.warnings > 0 then
div [ class "logs-container", class "-pipeline" ]
[ table
[ class "logs-table"
, class "pipeline"
]
[ div [ class "header" ]
[ span []
[ text "Warnings"
]
]
, if model.expand then
span [ class "tip" ]
[ small []
[ text "note: this pipeline is expanded, the line numbers shown only apply to the base pipeline configuration, they are not accurate when viewing an expanded pipeline." ]
]

else
text ""
, div [ class "warnings" ] <|
List.map (viewWarningAsLogLine model shared) <|
List.sort p.warnings
]
]

else
text ""

_ ->
text ""
, div [ class "pipeline" ]
[ case model.templates of
RemoteData.Success templates ->
if not <| Dict.isEmpty templates then
Expand Down Expand Up @@ -566,6 +606,7 @@ view shared route model =
[ div [ class "header" ]
[ span []
[ text "Pipeline Configuration"
, RemoteData.unwrap (text "") (\p -> span [ class "commit" ] [ text p.commit ]) model.pipeline
]
]
, div [ class "actions" ]
Expand All @@ -576,7 +617,7 @@ view shared route model =
RemoteData.Success pipeline ->
if String.length pipeline.decodedData > 0 then
div [ class "logs", Util.testAttribute "pipeline-configuration-data" ] <|
viewLines pipeline model.focus shared.shift
viewLines pipeline model.expand model.focus shared.shift

else
div [ class "no-pipeline" ] [ small [] [ code [] [ text "The pipeline found for this build/ref contains no data." ] ] ]
Expand Down Expand Up @@ -660,17 +701,19 @@ viewTemplate ( _, t ) =

{-| viewLines : creates a list of lines for a template.
-}
viewLines : Vela.PipelineConfig -> Focus.Focus -> Bool -> List (Html Msg)
viewLines config focus shift =
viewLines : Vela.PipelineConfig -> Bool -> Focus.Focus -> Bool -> List (Html Msg)
viewLines config expand focus shiftKeyDown =
config.decodedData
|> Utils.Ansi.decodeAnsi
|> Array.indexedMap
(\idx line ->
Just <|
viewLine
shift
expand
shiftKeyDown
(idx + 1)
(Just line)
(Dict.get (idx + 1) config.lineWarnings)
focus
)
|> Array.toList
Expand All @@ -679,11 +722,12 @@ viewLines config focus shift =

{-| viewLine : creates a numbered, linkable line for a template.
-}
viewLine : Bool -> Int -> Maybe Ansi.Log.Line -> Focus.Focus -> Html Msg
viewLine shiftKeyDown lineNumber line focus =
viewLine : Bool -> Bool -> Int -> Maybe Ansi.Log.Line -> Maybe (List String) -> Focus.Focus -> Html Msg
viewLine expand shiftKeyDown lineNumber line warnings focus =
tr
[ id <| String.fromInt lineNumber
, class "line"
, Maybe.Extra.unwrap (class "") (\_ -> class "-warning") warnings
]
[ case line of
Just l ->
Expand All @@ -692,7 +736,16 @@ viewLine shiftKeyDown lineNumber line focus =
, Util.testAttribute <| String.join "-" [ "config", "line", String.fromInt lineNumber ]
, class <| Focus.lineRangeStyles Nothing lineNumber focus
]
[ td []
[ td
[ class "annotation"
, warnings
|> Maybe.Extra.filter (\_ -> not expand)
|> Maybe.Extra.unwrap (class "-hide") (\_ -> class "-show")
]
[ Components.Svgs.annotationCircle "-warning" ]
, td
[ Html.Attributes.title <| "Jump to line " ++ String.fromInt lineNumber
]
[ button
[ Util.onClickPreventDefault <|
PushUrlHash
Expand All @@ -709,10 +762,11 @@ viewLine shiftKeyDown lineNumber line focus =
, class "button"
, class "-link"
, attribute "aria-label" "focus this line"
, title <| "Focus line " ++ String.fromInt lineNumber
]
[ span [] [ text <| String.fromInt lineNumber ] ]
]
, td [ class "break-text", class "overflow-auto" ]
, td [ class "break-text", class "overflow-auto", class "line-content" ]
[ code [ Util.testAttribute <| String.join "-" [ "config", "data", String.fromInt lineNumber ] ]
[ Ansi.Log.viewLine l
]
Expand All @@ -722,3 +776,60 @@ viewLine shiftKeyDown lineNumber line focus =
Nothing ->
text ""
]


{-| viewWarningAsLogLine : renders a warning as a log line.
-}
viewWarningAsLogLine : Model -> Shared.Model -> String -> Html Msg
viewWarningAsLogLine model shared warning =
let
( maybeLineNumber, content ) =
Vela.lineNumberWarningfromWarningString warning
in
tr [ class "warning" ]
[ td [ class "annotation", class "-show" ]
[ Components.Svgs.annotationCircle "-warning"
]
, td []
[ case maybeLineNumber of
Just lineNumber ->
let
-- control focus behaviour so that the button always snaps to the right page location
focusChanged =
Maybe.Extra.unwrap False (\a -> a == lineNumber) model.focus.a
in
button
[ Util.onClickPreventDefault <|
if model.expand then
NoOp

else if focusChanged then
FocusOn { target = Focus.toDomTarget { group = Nothing, a = Just lineNumber, b = Nothing } }

else
PushUrlHash
{ hash =
Focus.toString <| Focus.updateLineRange shared.shift Nothing lineNumber model.focus
}
, Util.testAttribute <| String.join "-" [ "warning", "line", "num", String.fromInt lineNumber ]
, class "button"
, class "-link"
, if model.expand then
class "-disabled"

else
class ""
, class "line-number"
, attribute "aria-label" "jump to this line"
, title <| "Jump to line " ++ String.fromInt lineNumber
, disabled model.expand
]
[ span [] [ text <| String.fromInt lineNumber ] ]

Nothing ->
span [ class "no-line-number" ] [ text "-" ]
]
, td [ class "line-content" ]
[ text content
]
]
Loading
Loading