fix notification implementation and tests

package.json

@@ -104,5 +104,8 @@
 	"engines": {
 		"yarn": "^1.15.2",
 		"node": "^18.16.0"
+	},
+	"dependencies": {
+		"yarn": "^1.22.19"
 	}
 }

packages/app/obojobo-document-engine/src/scripts/viewer/components/notification.scss

@@ -21,6 +21,10 @@ p {
 	border-width: 0.5em;
 }
 
+.notification-banner.hidden {
+	display: none;
+}
+
 .notification-banner:hover {
 	background-color: $focus-pink;
 	color: $poppin-pink;
@@ -45,6 +49,7 @@ p {
 	background-color: $white;
 	border: none;
 	border-radius: 0.5em;
+	font-size: 0.8cm;
 }
 
 .notification-exit-button:hover {

packages/app/obojobo-express/__tests__/express_current_user.test.js

@@ -3,14 +3,28 @@ const userFunctions = ['setCurrentUser', 'getCurrentUser', 'requireCurrentUser']
 
 jest.mock('test_node')
 jest.mock('../server/models/user')
+jest.mock('../server/viewer/viewer_notification_state')
+
+const viewerNotificationState = require('../server/viewer/viewer_notification_state')
+
+jest.mock('../server/viewer/viewer_state', () => ({
+	get: jest.fn()
+}))
+
+const viewerState = require('../server/viewer/viewer_state')
 
 describe('current user middleware', () => {
 	beforeAll(() => {})
 	afterAll(() => {})
 	beforeEach(() => {
+		jest.clearAllMocks()
 		mockArgs = (() => {
-			const res = {}
-			const req = { session: {} }
+			const res = {
+				cookie: jest.fn()
+			}
+			const req = {
+				session: {}
+			}
 			const mockJson = jest.fn().mockImplementation(() => {
 				return true
 			})
@@ -210,4 +224,68 @@ describe('current user middleware', () => {
 		})
 		return expect(req.saveSessionPromise()).rejects.toEqual('mock-error')
 	})
+	test('getNotifications sets notifications in cookies when notifications are available', async () => {
+		expect.assertions(6)
+
+		const { req, res } = mockArgs
+		const User = oboRequire('server/models/user')
+		const mockUser = new User({ id: 8, lastLogin: '2019-01-01' })
+		User.fetchById = jest.fn().mockResolvedValue(mockUser)
+		req.currentUserId = mockUser.id
+		req.currentUser = mockUser
+		req.currentUser.lastLogin = mockUser.lastLogin
+
+		const mockNotifications = [
+			{ id: 1, title: 'Notification 1', text: 'Message 1' },
+			{ id: 2, title: 'Notification 2', text: 'Message 2' }
+		]
+		//simulate what would be added to the cookie
+		const mockNotificationsToCookie = [
+			{ title: 'Notification 1', text: 'Message 1' },
+			{ title: 'Notification 2', text: 'Message 2' }
+		]
+
+		viewerState.get.mockResolvedValueOnce(req.currentUserId)
+		viewerNotificationState.getRecentNotifications.mockResolvedValueOnce(
+			mockNotifications.map(n => ({ id: n.id }))
+		)
+		viewerNotificationState.getNotifications.mockResolvedValueOnce(mockNotifications)
+
+		return req.getNotifications(req, res).then(() => {
+			const today = new Date()
+			expect(viewerState.get).toHaveBeenCalledWith(8)
+			expect(viewerNotificationState.getRecentNotifications).toHaveBeenCalled()
+			expect(viewerNotificationState.getNotifications).toHaveBeenCalledWith([1, 2])
+
+			expect(res.cookie).toHaveBeenCalledWith(
+				'notifications',
+				JSON.stringify(mockNotificationsToCookie)
+			)
+			expect(req.currentUser.lastLogin).toStrictEqual(today)
+			expect(viewerNotificationState.setLastLogin).toHaveBeenCalledWith(8, today)
+		})
+	})
+	test('getNotifications returns empty when there are no notifications', async () => {
+		expect.assertions(6)
+		const { req, res } = mockArgs
+		const User = oboRequire('server/models/user')
+		const mockUser = new User({ id: 8, lastLogin: '2019-01-01' })
+		User.fetchById = jest.fn().mockResolvedValue(mockUser)
+		req.currentUserId = mockUser.id
+		req.currentUser = mockUser
+		req.currentUser.lastLogin = mockUser.lastLogin
+
+		viewerState.get.mockResolvedValueOnce(req.currentUserId)
+		viewerNotificationState.getRecentNotifications.mockResolvedValueOnce(null)
+
+		return req.getNotifications(req, res).then(() => {
+			const today = new Date()
+			expect(viewerState.get).toHaveBeenCalledWith(8)
+			expect(viewerNotificationState.getRecentNotifications).toHaveBeenCalled() //With(req.currentUser.lastLogin)
+			expect(viewerNotificationState.getNotifications).not.toHaveBeenCalled()
+			expect(res.cookie).not.toHaveBeenCalled()
+			expect(req.currentUser.lastLogin).toStrictEqual(today)
+			expect(viewerNotificationState.setLastLogin).toHaveBeenCalledWith(8, today)
+		})
+	})
 })

packages/app/obojobo-express/__tests__/viewer_notification_state.test.js

@@ -1,41 +1,65 @@
 const db = require('../server/db')
 const {
 	getNotifications,
-	getRecentNotifications
+	getRecentNotifications,
+	setLastLogin
 } = require('../server/viewer/viewer_notification_state')
 
 jest.mock('../server/db')
+describe('db', () => {
+	beforeEach(() => {
+		jest.resetAllMocks()
+		jest.resetModules()
+	})
 
-describe('getNotifications', () => {
-	test('returns notifications when passed ids', async () => {
+	test('returns notifications when passed ids', () => {
 		const fakeNotifications = [
 			{ title: 'Notification 1', text: 'This is notification 1' },
 			{ title: 'Notification 2', text: 'This is notification 2' }
 		]
 		db.manyOrNone.mockResolvedValue(fakeNotifications)
 
-		const result = await getNotifications([1, 2])
-
-		expect(result).toEqual(fakeNotifications)
-		expect(db.manyOrNone).toHaveBeenCalledWith(expect.any(String), { ids: [1, 2] })
+		return getNotifications([1, 2]).then(result => {
+			expect(result).toEqual(fakeNotifications)
+			expect(db.manyOrNone).toHaveBeenCalledWith(expect.any(String), { ids: [1, 2] })
+		})
 	})
 
-	test('returns undefined when passed ids as 0', async () => {
-		const result = await getNotifications(0)
-
-		expect(result).toBeUndefined()
-		//expect(db.manyOrNone).not.toHaveBeenCalled(1)
+	test('returns undefined when passed ids as 0', () => {
+		return expect(getNotifications(0)).toBeUndefined()
 	})
-})
 
-describe('getRecentNotifications', () => {
-	test('returns notifications created after a given date', async () => {
+	test('returns notifications created after a given date', () => {
 		const fakeNotifications = [{ id: 1 }, { id: 2 }]
 		db.manyOrNone.mockResolvedValue(fakeNotifications)
 
-		const result = await getRecentNotifications('2022-01-01')
+		return getRecentNotifications('2022-01-01').then(result => {
+			expect(result).toEqual(fakeNotifications)
+			expect(db.manyOrNone).toHaveBeenCalledWith(expect.any(String), { date: '2022-01-01' })
+		})
+	})
+
+	test('should insert a new record if the user does not exist', () => {
+		db.none.mockResolvedValue()
+
+		const userId = 1
+		const today = '2023-09-13'
+
+		return setLastLogin(userId, today).then(() => {
+			expect(db.none).toHaveBeenCalledWith(expect.stringContaining('INSERT INTO users'), {
+				userId,
+				today
+			})
+		})
+	})
+
+	test('should handle other errors from db.none', () => {
+		const errorMessage = 'Database error'
+		db.none.mockRejectedValue(new Error(errorMessage))
+
+		const userId = 1
+		const today = '2023-09-13'
 
-		expect(result).toEqual(fakeNotifications)
-		expect(db.manyOrNone).toHaveBeenCalledWith(expect.any(String), { date: '2022-01-01' })
+		return expect(setLastLogin(userId, today)).rejects.toThrow(errorMessage)
 	})
 })

packages/app/obojobo-express/server/express_current_user.js

@@ -58,16 +58,21 @@ const saveSessionPromise = req => {
 	})
 }
 
-const getNotifications = (req, res) => {
-	return Promise.all([viewerState.get(req.currentUser.id)])
+//retrieve notifications from the database and set them in the cookie
+const getNotifications = async (req, res) => {
+	return Promise.all([viewerState.get(req.currentUserId)])
 		.then(() => viewerNotificationState.getRecentNotifications(req.currentUser.lastLogin))
 		.then(result => {
 			if (result) {
 				return result.map(notifications => notifications.id)
 			}
-			return 0
+			return [0]
+		})
+		.then(ids => {
+			if (ids.some(id => id !== 0)) {
+				return viewerNotificationState.getNotifications(ids.filter(id => id !== 0))
+			}
 		})
-		.then(ids => viewerNotificationState.getNotifications(ids))
 		.then(result => {
 			if (result) {
 				const parsedNotifications = result.map(notifications => ({
@@ -76,16 +81,13 @@ const getNotifications = (req, res) => {
 				}))
 				res.cookie('notifications', JSON.stringify(parsedNotifications))
 			}
+			return 0
 		})
-	//reset last login not working rn
-	/*
 		.then(() => {
 			const today = new Date()
-			//console.log(today)
 			req.currentUser.lastLogin = today
 			viewerNotificationState.setLastLogin(req.currentUser.id, today)
 		})
-		*/
 }
 
 module.exports = (req, res, next) => {

packages/app/obojobo-express/server/routes/api/visits.js

@@ -91,7 +91,6 @@ router
 					visitId: req.currentVisit.id
 				})
 			})
-			.then(req.getNotifications())
 
 			.then(() => {
 				logger.log(

packages/app/obojobo-express/server/viewer/viewer_notification_state.js

@@ -33,10 +33,10 @@ function getRecentNotifications(date) {
 function setLastLogin(userId, today) {
 	return db.none(
 		`
-			INSERT INTO users (last_login)
-			VALUES ($[today])]
-			ON CONFLICT (last_login) DO UPDATE
-			WHERE id == $[userId]
+			INSERT INTO users (id, last_login)
+			VALUES ($[userId], $[today])
+			ON CONFLICT (id) DO UPDATE
+			SET last_login = EXCLUDED.last_login
 		`,
 		{
 			userId,

packages/app/obojobo-repository/server/routes/dashboard.js

@@ -43,14 +43,17 @@ const renderDashboard = (req, res, options) => {
 	let moduleCount = 0
 	let pageTitle = 'Dashboard'
 
-	return getUserModuleCount(req.currentUser.id)
+	return req
+		.getNotifications(req, res)
+		.then(() => {
+			return getUserModuleCount(req.currentUser.id)
+		})
 		.then(count => {
 			moduleCount = count
 			return CollectionSummary.fetchByUserId(req.currentUser.id)
 		})
 		.then(collections => {
 			myCollections = collections
-
 			switch (options.mode) {
 				case MODE_COLLECTION:
 					pageTitle = 'View Collection'

packages/app/obojobo-repository/server/routes/dashboard.test.js

@@ -18,6 +18,7 @@ jest.setTimeout(10000) // extend test timeout?
 
 // override requireCurrentUser for tests to provide our own user
 let mockCurrentUser
+let mockNotifications
 
 jest.mock('obojobo-express/server/express_current_user', () => (req, res, next) => {
 	req.requireCurrentUser = () => {
@@ -28,6 +29,10 @@ jest.mock('obojobo-express/server/express_current_user', () => (req, res, next)
 		req.currentUser = mockCurrentUser
 		return Promise.resolve(mockCurrentUser)
 	}
+	req.getNotifications = () => {
+		return Promise.resolve(mockNotifications)
+	}
+
 	next()
 })
 
@@ -124,6 +129,8 @@ describe('repository dashboard route', () => {
 			id: 99,
 			hasPermission: perm => perm === 'canPreviewDrafts'
 		}
+		mockNotifications = []
+		//getNotifications = require('obojobo-express/server/express_current_user').getNotifications
 		CollectionSummary = require('../models/collection_summary')
 		DraftSummary = require('../models/draft_summary')
 		CountServices = require('../services/count')
@@ -154,8 +161,6 @@ describe('repository dashboard route', () => {
 	}
 
 	test('get /dashboard sends the correct props to the Dashboard component', () => {
-		expect.hasAssertions()
-
 		CountServices.getUserModuleCount.mockResolvedValueOnce(5)
 
 		CollectionSummary.fetchByUserId = jest.fn()
@@ -184,7 +189,6 @@ describe('repository dashboard route', () => {
 				expect(DraftSummary.fetchAllInCollection).not.toHaveBeenCalled()
 				expect(DraftSummary.fetchByUserId).not.toHaveBeenCalled()
 				expect(DraftSummary.fetchDeletedByUserId).not.toHaveBeenCalled()
-
 				expect(mockDashboardComponent).toHaveBeenCalledTimes(1)
 				expect(mockDashboardComponentConstructor).toHaveBeenCalledWith({
 					title: 'Dashboard',
@@ -309,6 +313,11 @@ describe('repository dashboard route', () => {
 	test('get /collections/:nameOrId sends the correct props to the Dashboard component with cookies set and the collection exists and the user owns the collection', () => {
 		expect.hasAssertions()
 
+		/*const req = {
+			getNotifications: jest.fn().mockResolvedValue(mockNotifications),
+			currentUser: mockCurrentUser,  // Assuming you have a mockCurrentUser defined
+		}*/
+
 		const mockShortToUUID = jest.fn()
 		mockShortToUUID.mockReturnValue('mockCollectionLongId')
 		short.mockReturnValue({
@@ -435,6 +444,10 @@ describe('repository dashboard route', () => {
 
 	test('get /collections/:nameOrId sends the correct response when the user owns the collection but a database error occurs', () => {
 		expect.hasAssertions()
+		/*const req = {
+			getNotifications: jest.fn().mockResolvedValue(mockNotifications),
+			currentUser: mockCurrentUser,  // Assuming you have a mockCurrentUser defined
+		}*/
 
 		const mockShortToUUID = jest.fn()
 		mockShortToUUID.mockReturnValue('mockCollectionLongId')
@@ -460,6 +473,7 @@ describe('repository dashboard route', () => {
 			.get(path)
 			.set('cookie', [''])
 			.then(response => {
+				//expect(req.getNotifications).toHaveBeenCalledTimes(1)
 				expect(mockShortToUUID).toHaveBeenCalledTimes(1)
 				expect(mockShortToUUID).toHaveBeenCalledWith('mockCollectionShortId')
 
@@ -488,6 +502,10 @@ describe('repository dashboard route', () => {
 
 	test('get /dashboard/deleted sends the correct props to the Dashboard component with cookies set', () => {
 		expect.hasAssertions()
+		/*const req = {
+			getNotifications: jest.fn().mockResolvedValue(mockNotifications),
+			currentUser: mockCurrentUser, 
+		} */
 
 		CountServices.getUserModuleCount.mockResolvedValueOnce(5)
 
@@ -503,6 +521,7 @@ describe('repository dashboard route', () => {
 			.get('/dashboard/deleted')
 			.set('cookie', [generateCookie('module', 'dashboard/deleted', 'last updated')])
 			.then(response => {
+				//expect(req.getNotifications).toHaveBeenCalledTimes(1)
 				expect(CountServices.getUserModuleCount).toHaveBeenCalledTimes(1)
 				expect(CountServices.getUserModuleCount).toHaveBeenCalledWith(mockCurrentUser.id)