Skip to content
This repository has been archived by the owner on May 30, 2022. It is now read-only.

Fix health container numbers and pagination numbers #725

Merged
merged 7 commits into from
Jan 24, 2022
Merged

Fix health container numbers and pagination numbers #725

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c8c5678
Visualize the health container and pagination number properly
arbulu89 Jan 19, 2022
e222e35
Filter the hosts properly by health
arbulu89 Jan 19, 2022
b1ad59a
Filter the clusters properly by health
arbulu89 Jan 20, 2022
288eca0
Update the tests to use the new filters
arbulu89 Jan 20, 2022
730fd83
Rename paginated lists variable names
arbulu89 Jan 20, 2022
1c80b6e
Move last checks result query to checks service
arbulu89 Jan 20, 2022
e69fbba
Improve hosts filtering and pagination e2e tests
arbulu89 Jan 20, 2022
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
31 changes: 21 additions & 10 deletions test/e2e/cypress/integration/hosts_overview.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ context('Hosts Overview', () => {
before(() => {
cy.resetDatabase()
cy.loadScenario('healthy-27-node-SAP-cluster')

cy.task('startAgentHeartbeat', agents())
cy.visit('/');
cy.navigateToItem('Hosts')
Expand All @@ -16,6 +16,12 @@ context('Hosts Overview', () => {
it('should show 10 of the 27 registered hosts with default pagination settings', () => {
cy.get('.tn-hostname').its('length').should('eq', 10)
})
it('should show 27 as total items in the pagination controls', () => {
cy.get('.pagination-count').should('contain', '27 items')
})
it('should have 5 pages', () => {
cy.get('.page-item').its('length').should('eq', 5)
})
it('should show all of the all 27 registered hosts when increasing pagination limit to 100', () => {
cy.reloadList('hosts', 100)
cy.get('.tn-hostname').its('length').should('eq', 27)
Expand All @@ -34,17 +40,16 @@ context('Hosts Overview', () => {

describe('Health Detection', () => {
describe('Health Container shows the health overview of the entire cluster', () => {
it('should show health status of the first 10 visible hosts', () => {
cy.log("This test needs to be removed in favor of having the Health Container showing information of the entire cluster")
it('should show health status of the entire cluster of 27 hosts with partial pagination', () => {
cy.reloadList('hosts', 10)
cy.get('.health-container .health-passing').should('contain', 10)
cy.get('.health-container .health-passing').should('contain', 27)
})
it('should show health status of the entire cluster of 27 hosts', () => {
cy.reloadList('hosts', 100)
cy.get('.health-container .health-passing').should('contain', 27)
})
})

describe('Detected hosts Health matches deployed server status', () => {
it('all 27 hosts in the cluster should be up', () => {
availableHosts.forEach((hostName) => cy.get(`#host-${hostName} > .row-status > i`).should('have.class', 'text-success'))
Expand Down Expand Up @@ -93,7 +98,7 @@ context('Hosts Overview', () => {
cy.wait('@resetFilter')
}

describe('Filtering by health', () => {
describe('Filtering by health', () => {
before(() => {
cy.get('.tn-filters > :nth-child(2) > .btn').click()
})
Expand All @@ -104,12 +109,14 @@ context('Hosts Overview', () => {
]
healthScenarios.forEach(([health, expectedHostsWithThisHealth], index) => {
it(`should show ${expectedHostsWithThisHealth || 'an empty list of'} hosts when filtering by health '${health}'`, () => {
cy.intercept('GET', `/hosts?per_page=100&health=${health}`).as('filterByHealthStatus')
cy.intercept('GET', `/hosts?per_page=100&health=${health}`).as('filterByHealthStatus')
const selectedOption = `#bs-select-1-${index}`
cy.get(selectedOption).click()
cy.wait('@filterByHealthStatus').then(() => {
expectedHostsWithThisHealth == 0 && cy.get('.table.eos-table').contains('There are currently no records to be shown')
expectedHostsWithThisHealth > 0 && cy.get('.tn-hostname').its('length').should('eq', expectedHostsWithThisHealth)
cy.get('.pagination-count').should('contain', `${expectedHostsWithThisHealth} items`)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a suggestion for the future:

This could be a custom cy command https://docs.cypress.io/api/cypress-api/custom-commands, and we can think of it as a custom assertion for pagination.

Something along these lines:

Cypress.Commands.add('paginationShouldMatch', (itemsCount) => {
    cy.get('.pagination-count').should('contain', `${itemsCount} items`)
    cy.get('.page-item').its('length').should('eq', Math.ceil(itemsCount/100)+2)
})

which then becomes accessible as

cy.paginationShouldMatch(expectedHostsWithThisHealth)

cy.paginationShouldMatch(expectedRelatedHosts)

cy.paginationShouldMatch(expectedTaggedHosts)

in the different variants.

Look, don't bother changing it now, as I understand it wouldn't add much value right now 😄
Just wanted to share the cypress concept of custom command that might become more and more useful as we go.

cy.get('.page-item').its('length').should('eq', Math.ceil(expectedHostsWithThisHealth/100)+2)
resetFilter(selectedOption)
})
})
Expand All @@ -127,15 +134,17 @@ context('Hosts Overview', () => {
['NWD', 4],
['NWP', 4],
['NWQ', 4],

]
SAPSystemsScenarios.forEach(([sapsystem, expectedRelatedHosts], index) => {
it(`should have ${expectedRelatedHosts} hosts related to SAP system '${sapsystem}'`, () => {
cy.intercept('GET', `/hosts?per_page=100&sids=${sapsystem}`).as('filterBySAPSystem')
cy.intercept('GET', `/hosts?per_page=100&sids=${sapsystem}`).as('filterBySAPSystem')
const selectedOption = `#bs-select-2-${index}`
cy.get(selectedOption).click()
cy.wait('@filterBySAPSystem').then(() => {
cy.get('.tn-hostname').its('length').should('eq', expectedRelatedHosts)
cy.get('.pagination-count').should('contain', `${expectedRelatedHosts} items`)
cy.get('.page-item').its('length').should('eq', Math.ceil(expectedRelatedHosts/100)+2)
resetFilter(selectedOption)
})
})
Expand All @@ -158,10 +167,12 @@ context('Hosts Overview', () => {
cy.get(selectedOption).click()
cy.wait('@filterByTags').then(() => {
cy.get('.tn-hostname').its('length').should('eq', expectedTaggedHosts)
cy.get('.pagination-count').should('contain', `${expectedTaggedHosts} items`)
cy.get('.page-item').its('length').should('eq', Math.ceil(expectedTaggedHosts/100)+2)
resetFilter(selectedOption)
})
})
})
})
})
});
});
17 changes: 9 additions & 8 deletions web/clusters.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,13 @@ func NewClusterListHandler(clustersService services.ClustersService) gin.Handler
Size: pageSize,
}

clusterList, err := clustersService.GetAll(clustersFilter, page)
paginatedClusterList, err := clustersService.GetAll(clustersFilter, page)
if err != nil {
_ = c.Error(err)
return
}

clusterList, err := clustersService.GetAll(clustersFilter, nil)
if err != nil {
_ = c.Error(err)
return
Expand Down Expand Up @@ -84,15 +90,10 @@ func NewClusterListHandler(clustersService services.ClustersService) gin.Handler
healthContainer := NewClustersHealthContainer(clusterList)
healthContainer.Layout = "horizontal"

count, err := clustersService.GetCount()
if err != nil {
_ = c.Error(err)
return
}
pagination := NewPagination(count, pageNumber, pageSize)
pagination := NewPagination(len(clusterList), pageNumber, pageSize)

c.HTML(http.StatusOK, "clusters.html.tmpl", gin.H{
"ClustersTable": clusterList,
"ClustersTable": paginatedClusterList,
"AppliedFilters": query,
"filterClusterNames": filterClusterNames,
"FilterClusterTypes": filterClusterTypes,
Expand Down
13 changes: 7 additions & 6 deletions web/hosts.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,36 +49,37 @@ func NewHostListHandler(hostsService services.HostsService) gin.HandlerFunc {
Size: pageSize,
}

hostList, err := hostsService.GetAll(hostsFilter, page)
paginatedHostList, err := hostsService.GetAll(hostsFilter, page)
if err != nil {
_ = c.Error(err)
return
}

filterSIDs, err := hostsService.GetAllSIDs()
hostList, err := hostsService.GetAll(hostsFilter, nil)
if err != nil {
_ = c.Error(err)
return
}

filterTags, err := hostsService.GetAllTags()
filterSIDs, err := hostsService.GetAllSIDs()
if err != nil {
_ = c.Error(err)
return
}

count, err := hostsService.GetCount()
filterTags, err := hostsService.GetAllTags()
if err != nil {
_ = c.Error(err)
return
}
pagination := NewPagination(count, pageNumber, pageSize)

pagination := NewPagination(len(hostList), pageNumber, pageSize)

hContainer := NewHostsHealthContainer(hostList)
hContainer.Layout = "horizontal"

c.HTML(http.StatusOK, "hosts.html.tmpl", gin.H{
"Hosts": hostList,
"Hosts": paginatedHostList,
"AppliedFilters": query,
"FilterSIDs": filterSIDs,
"FilterTags": filterTags,
Expand Down
53 changes: 53 additions & 0 deletions web/models/check_result.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,56 @@ type HostState struct {
Reachable bool `json:"reachable"`
Msg string `json:"msg"`
}

type AggregatedCheckData struct {
PassingCount int
WarningCount int
CriticalCount int
}

func (c *ChecksResult) GetAggregatedChecksResultByHost() map[string]*AggregatedCheckData {
aCheckDataByHost := make(map[string]*AggregatedCheckData)

for _, check := range c.Checks {
for hostName, host := range check.Hosts {
if _, ok := aCheckDataByHost[hostName]; !ok {
aCheckDataByHost[hostName] = &AggregatedCheckData{}
}
switch host.Result {
case CheckCritical:
aCheckDataByHost[hostName].CriticalCount += 1
case CheckWarning:
aCheckDataByHost[hostName].WarningCount += 1
case CheckPassing:
aCheckDataByHost[hostName].PassingCount += 1
}
}
}

return aCheckDataByHost
}

func (c *ChecksResult) GetAggregatedChecksResultByCluster() *AggregatedCheckData {
aCheckData := &AggregatedCheckData{}
aCheckDataByHost := c.GetAggregatedChecksResultByHost()

for _, aData := range aCheckDataByHost {
aCheckData.CriticalCount += aData.CriticalCount
aCheckData.WarningCount += aData.WarningCount
aCheckData.PassingCount += aData.PassingCount
}

return aCheckData
}

func (a *AggregatedCheckData) String() string {
if a.CriticalCount > 0 {
return CheckCritical
} else if a.WarningCount > 0 {
return CheckWarning
} else if a.PassingCount > 0 {
return CheckPassing
}

return CheckUndefined
}
15 changes: 0 additions & 15 deletions web/pagination.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package web

import (
"math"
"strconv"
)

const (
Expand All @@ -26,20 +25,6 @@ func getPageCount(items, perPage int) int {
return int((float64(items) + float64(perPage) - 1) / float64(perPage))
}

func NewPaginationWithStrings(items int, page, perPage string) *Pagination {
pageInt, err := strconv.Atoi(page)
if err != nil {
pageInt = defaultPageIndex
}

perPageInt, err := strconv.Atoi(perPage)
if err != nil {
perPageInt = defaultPerPage
}

return NewPagination(items, pageInt, perPageInt)
}

func NewPagination(items, page, perPage int) *Pagination {
pCount := getPageCount(items, perPage)

Expand Down
14 changes: 0 additions & 14 deletions web/pagination_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,20 +35,6 @@ func TestNewPagination(t *testing.T) {
assert.Equal(t, p.PageIndex, 5)
}

func TestNewPaginationWithStrings(t *testing.T) {
p := NewPaginationWithStrings(10, "1", "10")
assert.Equal(t, p.ItemCount, 10)
assert.Equal(t, p.PageIndex, 1)
assert.Equal(t, p.PerPage, 10)
assert.Equal(t, p.PageCount, 1)

p = NewPaginationWithStrings(10, "a", "b")
assert.Equal(t, p.ItemCount, 10)
assert.Equal(t, p.PageIndex, 1)
assert.Equal(t, p.PerPage, 10)
assert.Equal(t, p.PageCount, 1)
}

func TestGetCurrentPages(t *testing.T) {
p := NewPagination(111, 1, 10)
pages := p.GetCurrentPages()
Expand Down
26 changes: 14 additions & 12 deletions web/sap_systems.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,34 +32,35 @@ func NewSAPSystemListHandler(sapSystemsService services.SAPSystemsService) gin.H
Size: pageSize,
}

sapSystems, err := sapSystemsService.GetAllApplications(tagsFilter, page)
paginatedSapSystems, err := sapSystemsService.GetAllApplications(tagsFilter, page)
if err != nil {
_ = c.Error(err)
return
}

filterSIDs, err := sapSystemsService.GetAllApplicationsSIDs()
sapSystems, err := sapSystemsService.GetAllApplications(tagsFilter, nil)
if err != nil {
_ = c.Error(err)
return
}

filterTags, err := sapSystemsService.GetAllApplicationsTags()
filterSIDs, err := sapSystemsService.GetAllApplicationsSIDs()
if err != nil {
_ = c.Error(err)
return
}

count, err := sapSystemsService.GetApplicationsCount()
filterTags, err := sapSystemsService.GetAllApplicationsTags()
if err != nil {
_ = c.Error(err)
return
}
pagination := NewPagination(count, pageNumber, pageSize)

pagination := NewPagination(len(sapSystems), pageNumber, pageSize)

c.HTML(http.StatusOK, "sap_systems.html.tmpl", gin.H{
"Type": models.SAPSystemTypeApplication,
"SAPSystems": sapSystems,
"SAPSystems": paginatedSapSystems,
"AppliedFilters": query,
"FilterSIDs": filterSIDs,
"FilterTags": filterTags,
Expand Down Expand Up @@ -91,34 +92,35 @@ func NewHANADatabaseListHandler(sapSystemsService services.SAPSystemsService) gi
Size: pageSize,
}

databases, err := sapSystemsService.GetAllDatabases(tagsFilter, page)
paginatedDatabases, err := sapSystemsService.GetAllDatabases(tagsFilter, page)
if err != nil {
_ = c.Error(err)
return
}

filterSIDs, err := sapSystemsService.GetAllDatabasesSIDs()
databases, err := sapSystemsService.GetAllDatabases(tagsFilter, nil)
if err != nil {
_ = c.Error(err)
return
}

filterTags, err := sapSystemsService.GetAllDatabasesTags()
filterSIDs, err := sapSystemsService.GetAllDatabasesSIDs()
if err != nil {
_ = c.Error(err)
return
}

count, err := sapSystemsService.GetDatabasesCount()
filterTags, err := sapSystemsService.GetAllDatabasesTags()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above, I don't think we need to do the whole table fetching.

if err != nil {
_ = c.Error(err)
return
}
pagination := NewPagination(count, pageNumber, pageSize)

pagination := NewPagination(len(databases), pageNumber, pageSize)

c.HTML(http.StatusOK, "sap_systems.html.tmpl", gin.H{
"Type": models.SAPSystemTypeDatabase,
"SAPSystems": databases,
"SAPSystems": paginatedDatabases,
"AppliedFilters": query,
"FilterSIDs": filterSIDs,
"FilterTags": filterTags,
Expand Down
2 changes: 0 additions & 2 deletions web/sap_systems_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ func TestSAPSystemsListHandler(t *testing.T) {
AttachedDatabase: &models.SAPSystem{},
},
}, nil)
sapSystemsService.On("GetApplicationsCount").Return(1, nil)
sapSystemsService.On("GetAllApplicationsSIDs").Return([]string{"HA1"}, nil)
sapSystemsService.On("GetAllApplicationsTags").Return([]string{"tag1"}, nil)

Expand Down Expand Up @@ -139,7 +138,6 @@ func TestSAPDatabaseListHandler(t *testing.T) {
},
},
}, nil)
sapSystemsService.On("GetDatabasesCount").Return(1, nil)
sapSystemsService.On("GetAllDatabasesSIDs").Return([]string{"PRD"}, nil)
sapSystemsService.On("GetAllDatabasesTags").Return([]string{"tag1"}, nil)

Expand Down
Loading