-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
π feat: Configurable Retention Period for Temporary Chats #7917
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
Closed
ConstantTime
wants to merge
9
commits into
danny-avila:dev
from
ConstantTime:feat/configurable-temp-chat-retention
Closed
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
1d67f1a
feat: Add configurable retention period for temporary chats
ConstantTime 527a662
Addressing eslint errors
ConstantTime 26a908c
Fix: failing test due to missing registration
ConstantTime d89ce12
Update: variable name and use hours instead of days for chat retention
ConstantTime 10aede9
Merge branch 'main' into feat/configurable-temp-chat-retention
ConstantTime b2fcb4e
Addressing comments
ConstantTime fb87037
chore: fix import order in Conversation.js
danny-avila dcf7223
chore: import order in Message.js
danny-avila c97d068
chore: fix import order in config.ts
danny-avila File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,132 @@ | ||
const { | ||
DEFAULT_RETENTION_DAYS, | ||
MIN_RETENTION_DAYS, | ||
MAX_RETENTION_DAYS, | ||
getTempChatRetentionDays, | ||
createTempChatExpirationDate, | ||
} = require('../tempChatRetention'); | ||
|
||
describe('tempChatRetention', () => { | ||
const originalEnv = process.env; | ||
|
||
beforeEach(() => { | ||
jest.resetModules(); | ||
process.env = { ...originalEnv }; | ||
delete process.env.TEMP_CHAT_RETENTION_DAYS; | ||
}); | ||
|
||
afterAll(() => { | ||
process.env = originalEnv; | ||
}); | ||
|
||
describe('getTempChatRetentionDays', () => { | ||
it('should return default retention days when no config or env var is set', () => { | ||
const result = getTempChatRetentionDays(); | ||
expect(result).toBe(DEFAULT_RETENTION_DAYS); | ||
}); | ||
|
||
it('should use environment variable when set', () => { | ||
process.env.TEMP_CHAT_RETENTION_DAYS = '15'; | ||
const result = getTempChatRetentionDays(); | ||
expect(result).toBe(15); | ||
}); | ||
|
||
it('should use config value when set', () => { | ||
const config = { | ||
interface: { | ||
temporaryChatRetentionDays: 7, | ||
}, | ||
}; | ||
const result = getTempChatRetentionDays(config); | ||
expect(result).toBe(7); | ||
}); | ||
|
||
it('should prioritize config over environment variable', () => { | ||
process.env.TEMP_CHAT_RETENTION_DAYS = '15'; | ||
const config = { | ||
interface: { | ||
temporaryChatRetentionDays: 7, | ||
}, | ||
}; | ||
const result = getTempChatRetentionDays(config); | ||
expect(result).toBe(7); | ||
}); | ||
|
||
it('should enforce minimum retention period', () => { | ||
const config = { | ||
interface: { | ||
temporaryChatRetentionDays: 0, | ||
}, | ||
}; | ||
const result = getTempChatRetentionDays(config); | ||
expect(result).toBe(MIN_RETENTION_DAYS); | ||
}); | ||
|
||
it('should enforce maximum retention period', () => { | ||
const config = { | ||
interface: { | ||
temporaryChatRetentionDays: 400, | ||
}, | ||
}; | ||
const result = getTempChatRetentionDays(config); | ||
expect(result).toBe(MAX_RETENTION_DAYS); | ||
}); | ||
|
||
it('should handle invalid environment variable', () => { | ||
process.env.TEMP_CHAT_RETENTION_DAYS = 'invalid'; | ||
const result = getTempChatRetentionDays(); | ||
expect(result).toBe(DEFAULT_RETENTION_DAYS); | ||
}); | ||
|
||
it('should handle invalid config value', () => { | ||
const config = { | ||
interface: { | ||
temporaryChatRetentionDays: 'invalid', | ||
}, | ||
}; | ||
const result = getTempChatRetentionDays(config); | ||
expect(result).toBe(DEFAULT_RETENTION_DAYS); | ||
}); | ||
}); | ||
|
||
describe('createTempChatExpirationDate', () => { | ||
it('should create expiration date with default retention period', () => { | ||
const result = createTempChatExpirationDate(); | ||
|
||
const expectedDate = new Date(); | ||
expectedDate.setDate(expectedDate.getDate() + DEFAULT_RETENTION_DAYS); | ||
|
||
// Allow for small time differences in test execution | ||
const timeDiff = Math.abs(result.getTime() - expectedDate.getTime()); | ||
expect(timeDiff).toBeLessThan(1000); // Less than 1 second difference | ||
}); | ||
|
||
it('should create expiration date with custom retention period', () => { | ||
const config = { | ||
interface: { | ||
temporaryChatRetentionDays: 7, | ||
}, | ||
}; | ||
|
||
const result = createTempChatExpirationDate(config); | ||
|
||
const expectedDate = new Date(); | ||
expectedDate.setDate(expectedDate.getDate() + 7); | ||
|
||
// Allow for small time differences in test execution | ||
const timeDiff = Math.abs(result.getTime() - expectedDate.getTime()); | ||
expect(timeDiff).toBeLessThan(1000); // Less than 1 second difference | ||
}); | ||
|
||
it('should return a Date object', () => { | ||
const result = createTempChatExpirationDate(); | ||
expect(result).toBeInstanceOf(Date); | ||
}); | ||
|
||
it('should return a future date', () => { | ||
const now = new Date(); | ||
const result = createTempChatExpirationDate(); | ||
expect(result.getTime()).toBeGreaterThan(now.getTime()); | ||
}); | ||
}); | ||
}); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,84 @@ | ||
const { logger } = require('~/config'); | ||
|
||
/** | ||
* Default retention period for temporary chats in days | ||
*/ | ||
const DEFAULT_RETENTION_DAYS = 30; | ||
|
||
/** | ||
* Minimum allowed retention period in days | ||
*/ | ||
const MIN_RETENTION_DAYS = 1; | ||
|
||
/** | ||
* Maximum allowed retention period in days (1 year) | ||
*/ | ||
const MAX_RETENTION_DAYS = 365; | ||
|
||
/** | ||
* Gets the temporary chat retention period from environment variables or config | ||
* @param {TCustomConfig} [config] - The custom configuration object | ||
* @returns {number} The retention period in days | ||
*/ | ||
function getTempChatRetentionDays(config) { | ||
let retentionDays = DEFAULT_RETENTION_DAYS; | ||
|
||
// Check environment variable first | ||
if (process.env.TEMP_CHAT_RETENTION_DAYS) { | ||
const envValue = parseInt(process.env.TEMP_CHAT_RETENTION_DAYS, 10); | ||
if (!isNaN(envValue)) { | ||
retentionDays = envValue; | ||
} else { | ||
logger.warn( | ||
`Invalid TEMP_CHAT_RETENTION_DAYS environment variable: ${process.env.TEMP_CHAT_RETENTION_DAYS}. Using default: ${DEFAULT_RETENTION_DAYS} days.`, | ||
); | ||
} | ||
} | ||
|
||
// Check config file (takes precedence over environment variable) | ||
if (config?.interface?.temporaryChatRetentionDays !== undefined) { | ||
const configValue = config.interface.temporaryChatRetentionDays; | ||
if (typeof configValue === 'number' && !isNaN(configValue)) { | ||
retentionDays = configValue; | ||
} else { | ||
logger.warn( | ||
`Invalid temporaryChatRetentionDays in config: ${configValue}. Using ${retentionDays} days.`, | ||
); | ||
} | ||
} | ||
|
||
// Validate the retention period | ||
if (retentionDays < MIN_RETENTION_DAYS) { | ||
logger.warn( | ||
`Temporary chat retention period ${retentionDays} is below minimum ${MIN_RETENTION_DAYS} days. Using minimum value.`, | ||
); | ||
retentionDays = MIN_RETENTION_DAYS; | ||
} else if (retentionDays > MAX_RETENTION_DAYS) { | ||
logger.warn( | ||
`Temporary chat retention period ${retentionDays} exceeds maximum ${MAX_RETENTION_DAYS} days. Using maximum value.`, | ||
); | ||
retentionDays = MAX_RETENTION_DAYS; | ||
} | ||
|
||
return retentionDays; | ||
} | ||
|
||
/** | ||
* Creates an expiration date for temporary chats | ||
* @param {TCustomConfig} [config] - The custom configuration object | ||
* @returns {Date} The expiration date | ||
*/ | ||
function createTempChatExpirationDate(config) { | ||
const retentionDays = getTempChatRetentionDays(config); | ||
const expiredAt = new Date(); | ||
expiredAt.setDate(expiredAt.getDate() + retentionDays); | ||
return expiredAt; | ||
} | ||
|
||
module.exports = { | ||
DEFAULT_RETENTION_DAYS, | ||
MIN_RETENTION_DAYS, | ||
MAX_RETENTION_DAYS, | ||
getTempChatRetentionDays, | ||
createTempChatExpirationDate, | ||
}; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
documentation update needed: availableTools
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also, nitpick: rename to
temporaryChatRetention
and make unit to hours for more granular customizationThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Name update is done and documentation added here.