Skip to content

fix: unused user recentEntities#6636

Open
jaskfla wants to merge 6 commits intodevfrom
dedupe-recent-entities
Open

fix: unused user recentEntities#6636
jaskfla wants to merge 6 commits intodevfrom
dedupe-recent-entities

Conversation

@jaskfla
Copy link
Copy Markdown
Contributor

@jaskfla jaskfla commented Feb 12, 2026

No description provided.

@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

Hello @jaskfla, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses technical debt by eliminating an unused recentEntities field from user preferences. It streamlines the user model by removing obsolete logic, ensures data integrity across the database with a dedicated migration, and updates relevant tests to align with these changes, contributing to a cleaner and more maintainable codebase.

Highlights

  • User Preferences Cleanup: Removed the unused recentEntities field and its associated logic from user preference updates within the User model.
  • Database Migration: Introduced a new database migration to proactively remove the recentEntities field from existing user preferences in the user_account table, ensuring data consistency.
  • Test Suite Updates: Updated the EditUserAccounts test suite to reflect the removal of recentEntities and improved test robustness by using generateId() for dynamic test data and adding null checks.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • packages/central-server/src/tests/apiV2/userAccounts/EditUserAccounts.test.js
    • Updated imports to include generateId from @tupaia/database.
    • Added generateId() for dynamic project_id and country_id values in preference update tests.
    • Included expect(result).not.to.be.null assertions for retrieved user accounts.
    • Adjusted expected user preference objects to no longer include the recentEntities field.
    • Added /** @type {const} */ type assertions to DEFAULT_POLICY and BES_ADMIN_POLICY objects.
  • packages/database/src/core/migrations/20260212041501-CleanUpUnusedUserAccountPreferencesUserEntities-modifies-data.js
    • Added a new database migration file.
    • Implemented an up migration to remove the recentEntities key from the preferences JSONB column in the user_account table for all records where it exists.
  • packages/database/src/core/modelClasses/User/User.js
    • Removed the conditional logic that would clear the recentEntities field from user preferences when a project_id was updated.
Activity
  • No specific activity has been recorded for this pull request yet.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request starts the process of removing the unused recentEntities user preference by adding a data migration and updating related tests. However, the removal is incomplete, as several parts of the codebase still reference recentEntities. I've provided a detailed comment on where to find these references to ensure a complete cleanup and avoid leaving dead code. I also suggested an improvement to the migration file to make its down method explicitly irreversible, which is a good practice for maintainability.

I am having trouble creating individual review comments. Click here to see my feedback.

packages/database/src/core/modelClasses/User/User.js (346-349)

high

Removing this logic for recentEntities is a good step. However, the feature seems to be incompletely removed. Several other parts of the code still reference recentEntities:

  • USER_PREFERENCES_FIELDS in packages/constants/src/user.ts
  • The addRecentEntities.js file and its usage in User.js
  • getRecentEntities and getRecentEntityIds methods in User.js

To fully deprecate this feature and avoid dead code, these other occurrences should also be removed. Leaving them could lead to confusion and potential bugs if the 'unused' feature is accidentally used again.

packages/database/src/core/migrations/20260212041501-CleanUpUnusedUserAccountPreferencesUserEntities-modifies-data.js (25-27)

medium

The down migration is currently irreversible, which is acceptable for this kind of data cleanup. However, simply returning null can be ambiguous. It's better practice to make the irreversibility explicit by throwing an error. This prevents accidental rollbacks and clearly communicates the intent.

exports.down = function (db) {
  throw new Error('This migration is irreversible and cannot restore the "recentEntities" user preference.');
};

@jaskfla jaskfla force-pushed the dedupe-recent-entities branch from 6577500 to d85d4ab Compare February 16, 2026 10:49
Comment on lines 341 to -349
/** @type {UserAccountPreferences} */
const updatedPreferenceFields = updatedUserPreferences.reduce((obj, [key, value]) => {
obj[key] = value;
return obj;
}, preferences);
// If we change the selected project, we clear out the recent entities
if (updatedPreferenceFields.project_id) {
updatedPreferenceFields.recentEntities = {};
}
Copy link
Copy Markdown
Contributor Author

@jaskfla jaskfla Feb 16, 2026

Choose a reason for hiding this comment

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

The reducer’s initial accumulator value is the user’s existing preferences, which means project_id is almost always truthy

⟹ This if block almost always runs, but the property was always wrong. The real property is recent_entities, not recentEntities

⟹ But it also means that if we swapped this for recent_entities, it would get wiped very time we updat the user

Resolved in Slack that we actually want to preserve recent entities across projects anyway

Base automatically changed from rn-1717-test-four to rn-1545-epic-datatrak-offline March 2, 2026 04:44
Base automatically changed from rn-1545-epic-datatrak-offline to dev March 2, 2026 05:46
@jaskfla jaskfla force-pushed the dedupe-recent-entities branch from d85d4ab to 19f8ada Compare March 15, 2026 21:06
UPDATE user_account
SET preferences = preferences - 'recentEntities'
WHERE preferences ? 'recentEntities';
`);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Migration ? operator conflicts with parameter placeholder

High Severity

The JSONB ? operator in preferences ? 'recentEntities' will be interpreted by db.runSql as a parameter placeholder (as seen in other migrations like SET type = ? WHERE code = ?), causing the migration to fail at runtime. Using jsonb_exists(preferences, 'recentEntities') or an equivalent function form avoids the ambiguity.

Fix in Cursor Fix in Web

@jaskfla jaskfla force-pushed the dedupe-recent-entities branch from 19f8ada to f44d219 Compare March 19, 2026 00:35
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

@jaskfla jaskfla force-pushed the dedupe-recent-entities branch from 94eb9fd to 539e953 Compare April 14, 2026 02:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants