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.

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

datahub: Favorite star bug on page change #533

Merged
merged 9 commits into from
Jul 28, 2023
3 changes: 2 additions & 1 deletion .prettierignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

/dist
/coverage

/docs/.vitepress/cache
/docs/.vitepress/dist
# OpenAPI generated specs
**/spec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ describe('FavoriteStarComponent', () => {
let fixture: ComponentFixture<FavoriteStarComponent>
let authService: AuthService
let favoritesService: FavoritesService
let favoriteCountEl: HTMLElement
let favoriteCountHTMLEl: HTMLElement
let starToggle: StarToggleComponent

beforeEach(async () => {
Expand Down Expand Up @@ -65,7 +65,6 @@ describe('FavoriteStarComponent', () => {
favoritesService = TestBed.inject(FavoritesService)
fixture = TestBed.createComponent(FavoriteStarComponent)
component = fixture.componentInstance
component.record = { ...RECORDS_SUMMARY_FIXTURE[0], favoriteCount: 42 }
fixture.detectChanges()
starToggle = fixture.debugElement.query(
By.directive(StarToggleComponent)
Expand All @@ -76,146 +75,167 @@ describe('FavoriteStarComponent', () => {
expect(component).toBeTruthy()
})

describe('when a record has a favorite count', () => {
beforeEach(() => {
favoriteCountEl = fixture.debugElement.query(
By.css('.favorite-count')
).nativeElement
describe('Favorite count', () => {
describe('when a record has a favorite count', () => {
beforeEach(() => {
component.record = { ...RECORDS_SUMMARY_FIXTURE[0], favoriteCount: 42 }
fixture.detectChanges()
})
it('shows the amount of favorites on the record', () => {
favoriteCountHTMLEl = fixture.debugElement.query(
By.css('.favorite-count')
).nativeElement
expect(favoriteCountHTMLEl).toBeTruthy()
expect(favoriteCountHTMLEl.textContent).toEqual(
component.record.favoriteCount.toString()
)
})
})
it('shows the amount of favorites on the record', () => {
expect(favoriteCountEl).toBeTruthy()
expect(favoriteCountEl.textContent).toEqual(
component.record.favoriteCount.toString()
)
describe('when a record does not have a favorite count', () => {
beforeEach(() => {
component.record = { ...RECORDS_SUMMARY_FIXTURE[0] }
fixture.detectChanges()
})
it('does not show the amount of favorites on the record', () => {
const favoriteCountEl = fixture.debugElement.query(
By.css('.favorite-count')
)
expect(favoriteCountEl).toBeFalsy()
})
})
})
describe('when a record does not have a favorite count', () => {
describe('Favorite component availability', () => {
describe('when authenticated', () => {
it('star toggle is enabled', () => {
expect(starToggle.disabled).toBe(false)
})
it('does not create tippy tooltip', () => {
expect(tippy).not.toHaveBeenCalled()
})
})
describe('when not authenticated', () => {
beforeEach(() => {
;(authService as any).isAnonymous$.next(true)
fixture.detectChanges()
})
it('star toggle is disabled', () => {
expect(starToggle.disabled).toBe(true)
})
it('creates tippy tooltip', () => {
expect(tippy).toHaveBeenCalledWith(
expect.any(Object),
expect.objectContaining({
content: 'You can log in here',
allowHTML: true,
interactive: true,
zIndex: 40,
maxWidth: 250,
})
)
})
})
})
describe('On favorite click', () => {
beforeEach(() => {
component.record = { ...RECORDS_SUMMARY_FIXTURE[0] }
delete component.record.favoriteCount
component.record = { ...RECORDS_SUMMARY_FIXTURE[0], favoriteCount: 42 }
fixture.detectChanges()
})
it('does not show the amount of favorites on the record', () => {
expect(fixture.debugElement.query(By.css('.favorite-count'))).toBeFalsy()
describe('When the record is already in favorites', () => {
beforeEach(() => {
starToggle.newValue.emit(false)
fixture.detectChanges()
})
it('removes record from favorites', () => {
expect(favoritesService.removeFromFavorites).toHaveBeenCalledWith([
component.record.uuid,
])
expect(favoritesService.addToFavorites).not.toHaveBeenCalled()
})
})
describe('When the record is not in favorites', () => {
beforeEach(() => {
starToggle.newValue.emit(true)
fixture.detectChanges()
})
it('adds record to favorites', () => {
expect(favoritesService.addToFavorites).toHaveBeenCalledWith([
component.record.uuid,
])
expect(favoritesService.removeFromFavorites).not.toHaveBeenCalled()
})
})
})
describe('when authenticated', () => {
it('star toggle is enabled', () => {
expect(starToggle.disabled).toBe(false)

describe('On favorites array update', () => {
beforeEach(() => {
component.record = { ...RECORDS_SUMMARY_FIXTURE[0], favoriteCount: 42 }
fixture.detectChanges()
favoriteCountHTMLEl = fixture.debugElement.query(
By.css('.favorite-count')
).nativeElement
})
it('does not create tippy tooltip', () => {
expect(tippy).not.toHaveBeenCalled()
describe('When my record is part of the updates', () => {
beforeEach(() => {
;(favoritesService as any).myFavoritesUuid$.next([
component.record.uuid,
])
fixture.detectChanges()
})
it('increase record favorite count by one', () => {
expect(favoriteCountHTMLEl.textContent).toEqual(
(component.record.favoriteCount + 1).toString()
)
})
})
describe('on toggle state change', () => {
describe('When my record is not part of the updates', () => {
beforeEach(() => {
favoriteCountEl = fixture.debugElement.query(
By.css('.favorite-count')
).nativeElement
;(favoritesService as any).myFavoritesUuid$.next(['aaa', 'bbb'])
fixture.detectChanges()
})
describe('if record is not part of favorite', () => {
beforeEach(() => {
;(favoritesService as any).myFavoritesUuid$.next([
'aaa',
'bbb',
'ccc',
])
starToggle.newValue.emit(true)
fixture.detectChanges()
})
it('adds record to favorites', () => {
expect(favoritesService.addToFavorites).toHaveBeenCalledWith([
component.record.uuid,
])
expect(favoritesService.removeFromFavorites).not.toHaveBeenCalled()
})
it('increase record favorite count by one', () => {
expect(favoriteCountEl.textContent).toEqual(
(component.record.favoriteCount + 1).toString()
)
})
})
describe('if record is part of favorite', () => {
beforeEach(() => {
;(favoritesService as any).myFavoritesUuid$.next([
'aaa',
'bbb',
component.record.uuid,
])
starToggle.newValue.emit(false)
fixture.detectChanges()
})
it('removes record from favorites', () => {
expect(favoritesService.removeFromFavorites).toHaveBeenCalledWith([
component.record.uuid,
])
expect(favoritesService.addToFavorites).not.toHaveBeenCalled()
})
it('decrease record favorite count by one', () => {
expect(favoriteCountEl.textContent).toEqual(
(component.record.favoriteCount - 1).toString()
)
})
})
describe('two subsequent changes', () => {
beforeEach(() => {
;(favoritesService as any).myFavoritesUuid$.next([
'aaa',
'bbb',
component.record.uuid,
])
starToggle.newValue.emit(false)
starToggle.newValue.emit(true)
fixture.detectChanges()
})
it('removes and adds record to favorites', () => {
expect(favoritesService.removeFromFavorites).toHaveBeenCalledWith([
component.record.uuid,
])
expect(favoritesService.addToFavorites).toHaveBeenCalledWith([
component.record.uuid,
])
})
it('record favorite count stays the same', () => {
expect(favoriteCountEl.textContent).toEqual(
component.record.favoriteCount.toString()
)
})
})
describe('if favorite modification fails', () => {
beforeEach(() => {
favoritesService.addToFavorites = () => throwError('blargz')
starToggle.newValue.emit(true)
fixture.detectChanges()
})
it('does not change record favorite count', () => {
expect(favoriteCountEl.textContent).toEqual(
component.record.favoriteCount.toString()
)
})
it('increase record favorite count by one', () => {
expect(favoriteCountHTMLEl.textContent).toEqual(
component.record.favoriteCount.toString()
)
})
})
})
describe('when not authenticated', () => {

describe('two subsequent changes', () => {
beforeEach(() => {
component.record = { ...RECORDS_SUMMARY_FIXTURE[0], favoriteCount: 42 }
;(favoritesService as any).myFavoritesUuid$.next(['aaa'])
starToggle.newValue.emit(false)
starToggle.newValue.emit(true)
fixture.detectChanges()
})
it('removes and adds record to favorites', () => {
expect(favoritesService.removeFromFavorites).toHaveBeenCalledWith([
component.record.uuid,
])
expect(favoritesService.addToFavorites).toHaveBeenCalledWith([
component.record.uuid,
])
})
it('record favorite count stays the same', () => {
expect(favoriteCountHTMLEl.textContent).toEqual(
component.record.favoriteCount.toString()
)
})
})
describe('if favorite modification fails', () => {
beforeEach(() => {
;(authService as any).isAnonymous$.next(true)
component.record = { ...RECORDS_SUMMARY_FIXTURE[0], favoriteCount: 42 }
favoritesService.addToFavorites = () => throwError('blargz')
starToggle.newValue.emit(true)
fixture.detectChanges()
})
it('star toggle is disabled', () => {
expect(starToggle.disabled).toBe(true)
})
it('creates tippy tooltip', () => {
expect(tippy).toHaveBeenCalledWith(
expect.any(Object),
expect.objectContaining({
content: 'You can log in here',
allowHTML: true,
interactive: true,
zIndex: 40,
maxWidth: 250,
})
it('does not change record favorite count', () => {
expect(favoriteCountHTMLEl.textContent).toEqual(
component.record.favoriteCount.toString()
)
})
})

describe('unsubscribe', () => {
let unsubscribeSpy
beforeEach(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
} from '@angular/core'
import { FavoritesService } from '../favorites.service'
import { MetadataRecord } from '@geonetwork-ui/util/shared'
import { map } from 'rxjs/operators'
import { first, map, pairwise, take } from 'rxjs/operators'
import { AuthService } from '@geonetwork-ui/feature/auth'
import tippy from 'tippy.js'
import { TranslateService } from '@ngx-translate/core'
Expand Down Expand Up @@ -49,6 +49,7 @@ export class FavoriteStarComponent implements AfterViewInit, OnDestroy {
@ViewChild(StarToggleComponent, { read: ElementRef })
starToggleRef: ElementRef
subscription: Subscription
countSubscription: Subscription

get hasFavoriteCount() {
return this.favoriteCount !== null
Expand All @@ -74,10 +75,27 @@ export class FavoriteStarComponent implements AfterViewInit, OnDestroy {
})
}
})
this.countSubscription = this.favoritesService.myFavoritesUuid$
cmoinier marked this conversation as resolved.
Show resolved Hide resolved
.pipe(pairwise())
.subscribe(([oldFavs, newFavs]) => {
const editedFavs = (
oldFavs.length < newFavs.length
? newFavs.slice(-1)
: oldFavs.filter((fav) => !newFavs.includes(fav))
)[0]
if (this.hasFavoriteCount && editedFavs === this.record.uuid) {
if (newFavs.includes(editedFavs)) {
this.favoriteCount += 1
} else {
this.favoriteCount += -1
}
}
})
}

ngOnDestroy(): void {
this.subscription.unsubscribe()
Copy link
Member

Choose a reason for hiding this comment

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

Usually, one subscription is enough, you could use subscription.add() to all subscribes that you want to unsubscribe from.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I did try that. For some reason the subscription isn't working (as in, nothing happens, no update on directly clicked record nor on the other page) unless it's in its own subscription. I agree this is not the best way.

Copy link
Member

Choose a reason for hiding this comment

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

It should work, what did you try ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Tried the normal way : this.subscription.add() on this.favoritesService.myFavoritesUuid$.pipe() ......

this.countSubscription.unsubscribe()
}

toggleFavorite(isFavorite) {
Expand All @@ -87,9 +105,6 @@ export class FavoriteStarComponent implements AfterViewInit, OnDestroy {
: this.favoritesService.removeFromFavorites([this.record.uuid])
).subscribe({
complete: () => {
if (this.hasFavoriteCount) {
this.favoriteCount += isFavorite ? 1 : -1
}
this.loading = false
this.changeDetector.detectChanges()
},
Expand Down
Loading