feat: implement feature-role mapping #836
Conversation
WalkthroughAdds feature-role mapping: new FeatureRoleMapping model, queries, and four migrations (create table, seed feature, map roles, constraints). organization-feature service gains transactions, role validation and mapping management on create/update, and role-aware listing; controller forwards decoded token roles to the service. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant Controller as Controller (org-feature)
participant Service as Service (org-feature)
participant OrgFeatQ as OrgFeature Queries
participant FRMQ as FeatureRoleMapping Queries
Client->>Controller: GET /orgs/{org}/features (token with roles)
Controller->>Service: list(tenantCode, orgCode, userRoles)
alt userRoles include admin/org_admin/tenant_admin
Service->>OrgFeatQ: find enabled features (no role filter)
else Non-admin roles
Service->>OrgFeatQ: find enabled features
Service->>FRMQ: find mappings for tenant/org (filter by userRoles)
Service->>Service: intersect org features with mapped features
end
Service-->>Controller: features (processed)
Controller-->>Client: 200 OK + features
sequenceDiagram
autonumber
participant Client
participant Controller as Controller (org-feature)
participant Service as Service (org-feature)
participant RoleQ as Role Queries
participant FRMQ as FeatureRoleMapping Queries
Client->>Controller: POST/PUT /orgs/{org}/features (body.roles[])
Controller->>Service: create/update(featureData, roles)
Service->>Service: begin transaction
alt roles provided
Service->>RoleQ: fetch tenant roles (validate)
alt validation fails
Service-->>Controller: ROLE_NOT_FOUND (rollback)
else validation passes
Service->>FRMQ: delete existing mappings (update)
Service->>FRMQ: bulkCreate new mappings
end
end
Service->>Service: commit transaction
Service-->>Controller: success response
Controller-->>Client: HTTP response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/controllers/v1/organization-feature.js(1 hunks)src/database/migrations/20251002164809-create-feature-role-mapping-table.js(1 hunks)src/database/migrations/20251002164938-add-new-feature.js(1 hunks)src/database/migrations/20251002165109-map-roles-to-features.js(1 hunks)src/database/migrations/20251003155747-add-feature-role-mapping-constraints.js(1 hunks)src/database/models/Feature.js(1 hunks)src/database/models/featureRoleMapping.js(1 hunks)src/database/queries/featureRoleMapping.js(1 hunks)src/database/queries/organization-feature.js(2 hunks)src/services/organization-feature.js(6 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/controllers/**
⚙️ CodeRabbit configuration file
These are API controllers. Focus on request/response handling, security (auth middleware usage), and proper status codes.
Files:
src/controllers/v1/organization-feature.js
src/database/queries/**
⚙️ CodeRabbit configuration file
Review database queries for performance. Check for N+1 problems and ensure indexes can be used.
Files:
src/database/queries/featureRoleMapping.jssrc/database/queries/organization-feature.js
src/services/**
⚙️ CodeRabbit configuration file
This is core business logic. Please check for correctness, efficiency, and potential edge cases.
Files:
src/services/organization-feature.js
🧬 Code graph analysis (6)
src/database/models/featureRoleMapping.js (2)
src/database/queries/featureRoleMapping.js (1)
FeatureRoleMapping(8-8)src/database/queries/organization-feature.js (1)
FeatureRoleMapping(3-3)
src/database/queries/featureRoleMapping.js (3)
src/database/models/featureRoleMapping.js (1)
FeatureRoleMapping(2-35)src/database/queries/organization-feature.js (2)
FeatureRoleMapping(3-3)require(4-4)src/services/organization-feature.js (4)
require(14-14)require(16-16)filter(239-242)filter(335-339)
src/database/models/Feature.js (1)
src/database/queries/feature.js (1)
Feature(1-1)
src/services/organization-feature.js (2)
src/middlewares/authenticator.js (5)
require(15-15)require(18-18)require(19-19)common(11-11)roleQueries(13-13)src/controllers/v1/organization-feature.js (3)
common(10-10)tenantCode(69-69)orgCode(68-68)
src/database/migrations/20251002165109-map-roles-to-features.js (1)
src/services/organization-feature.js (2)
featureRoleMappingData(118-123)featureRoleMappingData(199-204)
src/database/queries/organization-feature.js (2)
src/database/models/featureRoleMapping.js (1)
FeatureRoleMapping(2-35)src/services/organization-feature.js (1)
roleTitles(273-273)
…e and update methods
…o featureRoleMap
…nsertion; improve organization feature mappings.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/database/migrations/20251002165109-map-roles-to-features.js (2)
69-115: Optimize the N+1 query pattern for tenant roles.The loop queries
user_rolesseparately for each organization (lines 74-80), creating an N+1 query pattern. For deployments with many organizations, this could significantly slow the migration.Refactor to fetch all roles once and group by tenant:
+ // Fetch all tenant roles upfront + const allTenantRoles = await queryInterface.sequelize.query( + 'SELECT title, tenant_code FROM user_roles WHERE deleted_at IS NULL', + { type: Sequelize.QueryTypes.SELECT } + ) + const rolesByTenant = new Map() + allTenantRoles.forEach((role) => { + if (!rolesByTenant.has(role.tenant_code)) { + rolesByTenant.set(role.tenant_code, []) + } + rolesByTenant.get(role.tenant_code).push(role.title) + }) + // Main loop: Process each organization to create feature-role mappings for (const org of uniqueOrgs) { const orgKey = `${org.organization_code}|${org.tenant_code}` const enabledFeatures = orgFeaturesMap.get(orgKey) || new Set() - // Fetch roles available for this tenant - const tenantRoles = await queryInterface.sequelize.query( - 'SELECT title FROM user_roles WHERE tenant_code = ? AND deleted_at IS NULL', - { - replacements: [org.tenant_code], - type: Sequelize.QueryTypes.SELECT, - } - ) - const tenantRoleTitles = tenantRoles.map((role) => role.title) + const tenantRoleTitles = rolesByTenant.get(org.tenant_code) || []
127-129: Consider explicit transaction wrapping for data integrity.The migration performs multiple queries and a final bulk insert without explicit transaction boundaries. If the insert fails, prior query results cannot be rolled back.
Wrap the migration logic in a transaction:
async up(queryInterface, Sequelize) { + const transaction = await queryInterface.sequelize.transaction() + try { // Fetch all enabled features for all organizations from the `organization_features` table. const organizationsFeatures = await queryInterface.sequelize.query( 'SELECT feature_code, tenant_code, organization_code FROM organization_features WHERE deleted_at IS NULL', - { type: Sequelize.QueryTypes.SELECT } + { type: Sequelize.QueryTypes.SELECT, transaction } ) // ... rest of the logic ... if (featureRoleMappingData.length > 0) { - await queryInterface.bulkInsert('feature_role_mapping', featureRoleMappingData) + await queryInterface.bulkInsert('feature_role_mapping', featureRoleMappingData, { transaction }) } + await transaction.commit() + } catch (error) { + await transaction.rollback() + throw error + } },
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/database/migrations/20251002164938-add-new-feature.js(1 hunks)src/database/migrations/20251002165109-map-roles-to-features.js(1 hunks)src/distributionColumns.sql(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/database/migrations/20251002164938-add-new-feature.js
🧰 Additional context used
🧬 Code graph analysis (1)
src/database/migrations/20251002165109-map-roles-to-features.js (1)
src/services/organization-feature.js (2)
featureRoleMappingData(120-125)featureRoleMappingData(204-209)
🔇 Additional comments (3)
src/distributionColumns.sql (1)
67-69: LGTM! Correct distribution strategy.The
feature_role_mappingtable is appropriately distributed bytenant_code, ensuring co-location with related tables likeorganization_features,user_roles, anduser_organizations. This will enable efficient joins for role-based feature queries.src/database/migrations/20251002165109-map-roles-to-features.js (2)
90-99: LGTM! Proper validation prevents orphaned mappings.The check ensures mappings are only created for features that are explicitly enabled for each organization. The logging of skipped mappings aids debugging and provides visibility into data quality.
19-35: Hardcoding in migration is appropriate for one-time setup; verify business confirmation and process handoff separately.The migration design is sound. It intelligently seeds role-feature mappings by respecting per-organization feature enablement and tenant-specific roles, with proper duplicate detection and rollback logic. Hardcoding this foundational data in a migration is standard practice—once persisted, the
FeatureRoleMappingmodel and queries enable flexible future updates.However, the previous reviewer's concerns about confirmation and QA notification are process issues outside code scope:
- Verify the role-feature assignments are correct per product requirements
- Confirm that UI/workflow impact was communicated to QA and affected teams (handled separately from this PR)
The code itself requires no changes.
src/database/migrations/20251002164809-create-feature-role-mapping-table.js
Show resolved
Hide resolved
… and updated_by fields
…redundancy in organization feature access checks.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/services/organization-feature.js (1)
187-208: Explicitly rollback transaction before early returns.The method has two early return paths (lines 188-192 and 203-207) that don't explicitly rollback the transaction. Add explicit rollback calls before returning.
// Return error if no record was updated if (updatedCount === 0) { + await transaction.rollback() return responses.failureResponse({ message: 'FAILED_TO_UPDATE_ORG_FEATURE', statusCode: httpStatusCode.bad_request, responseCode: 'CLIENT_ERROR', }) } // If roles are provided, update feature_role_mapping entries if (bodyData?.roles && Array.isArray(bodyData?.roles)) { // Validate that all requested roles exist for this tenant const validRoles = await roleQueries.findAll({ tenant_code: tokenInformation.tenant_code, title: { [Op.in]: bodyData.roles }, }) if (validRoles.length !== bodyData.roles.length) { + await transaction.rollback() return responses.failureResponse({ message: 'ROLE_NOT_FOUND', statusCode: httpStatusCode.bad_request, responseCode: 'CLIENT_ERROR', }) }
🧹 Nitpick comments (4)
src/database/migrations/20251002165109-map-roles-to-features.js (3)
17-17: Useconstfor immutable binding.The
defaultFeaturesarray is never reassigned, so declare it withconstinstead oflet.-let defaultFeatures = ['project', 'mentoring', 'survey', 'observation', 'reports', 'mitra', 'programs'] +const defaultFeatures = ['project', 'mentoring', 'survey', 'observation', 'reports', 'mitra', 'programs']
70-82: Optimize tenant role queries to avoid N+1 pattern.The migration queries
user_rolesinside the loop for each organization, resulting in N queries where N is the number of unique organizations. This significantly slows the migration.Consider fetching all tenant roles in a single query before the loop and grouping by tenant_code:
+ // Fetch all tenant roles once + const allTenantRoles = await queryInterface.sequelize.query( + 'SELECT title, tenant_code FROM user_roles WHERE deleted_at IS NULL', + { type: Sequelize.QueryTypes.SELECT } + ) + // Group roles by tenant_code + const rolesByTenant = new Map() + allTenantRoles.forEach(role => { + if (!rolesByTenant.has(role.tenant_code)) { + rolesByTenant.set(role.tenant_code, []) + } + rolesByTenant.get(role.tenant_code).push(role.title) + }) + for (const org of uniqueOrgs) { const orgKey = `${org.organization_code}|${org.tenant_code}` const enabledFeatures = orgFeaturesMap.get(orgKey) || new Set() - // Fetch roles available for this tenant - const tenantRoles = await queryInterface.sequelize.query( - 'SELECT title FROM user_roles WHERE tenant_code = ? AND deleted_at IS NULL', - { - replacements: [org.tenant_code], - type: Sequelize.QueryTypes.SELECT, - } - ) - const tenantRoleTitles = tenantRoles.map((role) => role.title) + const tenantRoleTitles = rolesByTenant.get(org.tenant_code) || []
5-133: Wrap migration operations in a transaction.The migration performs multiple database operations (queries and bulk insert) without a transaction. If the bulk insert fails partway through, the database could be left in an inconsistent state with some mappings created and others missing.
Wrap the up method in a transaction:
async up(queryInterface, Sequelize) { + const transaction = await queryInterface.sequelize.transaction() + try { // Fetch all enabled features for all organizations from the `organization_features` table. const organizationsFeatures = await queryInterface.sequelize.query( 'SELECT feature_code, tenant_code, organization_code FROM organization_features WHERE deleted_at IS NULL', - { type: Sequelize.QueryTypes.SELECT } + { type: Sequelize.QueryTypes.SELECT, transaction } ) // ... rest of the logic ... // Insert all valid mappings into the database if (featureRoleMappingData.length > 0) { - await queryInterface.bulkInsert('feature_role_mapping', featureRoleMappingData) + await queryInterface.bulkInsert('feature_role_mapping', featureRoleMappingData, { transaction }) } + await transaction.commit() + } catch (error) { + await transaction.rollback() + throw error + } },src/services/organization-feature.js (1)
296-302: Validate userRoles structure before accessing properties.The code assumes each element in
userRoleshas atitleproperty (line 298), but there's no validation of the array structure. IfuserRolescontains strings or objects without atitleproperty, the role extraction would fail silently, resulting in emptyroleTitlesand incorrect filtering.Add structure validation:
// Filter features based on user roles if (userRoles?.length > 0) { + // Validate userRoles structure + if (!Array.isArray(userRoles) || !userRoles.every(role => role?.title)) { + return responses.failureResponse({ + message: 'INVALID_USER_ROLES_FORMAT', + statusCode: httpStatusCode.bad_request, + responseCode: 'CLIENT_ERROR', + }) + } + // Extract role titles from user roles const roleTitles = userRoles.map((role) => role.title)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/constants/common.js(1 hunks)src/database/migrations/20251002164809-create-feature-role-mapping-table.js(1 hunks)src/database/migrations/20251002165109-map-roles-to-features.js(1 hunks)src/database/models/featureRoleMapping.js(1 hunks)src/services/organization-feature.js(9 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/database/migrations/20251002164809-create-feature-role-mapping-table.js
- src/database/models/featureRoleMapping.js
🧰 Additional context used
📓 Path-based instructions (1)
src/services/**
⚙️ CodeRabbit configuration file
This is core business logic. Please check for correctness, efficiency, and potential edge cases.
Files:
src/services/organization-feature.js
🧬 Code graph analysis (2)
src/services/organization-feature.js (2)
src/database/queries/organization-feature.js (1)
require(4-4)src/controllers/v1/organization-feature.js (3)
common(10-10)tenantCode(69-69)orgCode(68-68)
src/database/migrations/20251002165109-map-roles-to-features.js (1)
src/services/organization-feature.js (2)
featureRoleMappingData(120-127)featureRoleMappingData(222-229)
🔇 Additional comments (1)
src/constants/common.js (1)
45-45: LGTM! Clean constant addition.The new
TENANT_ADMIN_ROLEconstant follows existing conventions and is logically positioned in the role hierarchy.
…mline role handling
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/services/organization-feature.js (1)
188-194: Explicitly rollback transaction before early return.When no records are updated, the method returns without explicitly rolling back the transaction. Add an explicit rollback before returning the failure response.
Apply this diff:
// Return error if no record was updated if (updatedCount === 0) { + await transaction.rollback() return responses.failureResponse({ message: 'FAILED_TO_UPDATE_ORG_FEATURE', statusCode: httpStatusCode.bad_request, responseCode: 'CLIENT_ERROR', }) }
♻️ Duplicate comments (3)
src/services/organization-feature.js (3)
113-118: Explicitly rollback transaction before early return.When role validation fails, the method returns without explicitly rolling back the transaction. While Sequelize will eventually auto-rollback uncommitted transactions, explicit rollback improves clarity and ensures immediate cleanup.
Apply this diff:
const validRoleTitles = validRoles.map((role) => role.title) if (validRoleTitles.length === 0 || validRoleTitles.length !== bodyData?.roles?.length) { + await transaction.rollback() return responses.failureResponse({ message: 'ROLE_NOT_FOUND', statusCode: httpStatusCode.bad_request, responseCode: 'CLIENT_ERROR', }) }
314-353: Address security gap when no role mappings exist.The current implementation shows all organization features to non-admin users when
roleFeatureMappingsis empty or has no matches. This creates a security vulnerability when:
- Migrations haven't run yet
- New roles are added after migration
- Feature mappings are accidentally deleted
In these scenarios, non-admin users inappropriately gain access to all features.
Apply this diff to explicitly restrict access when no mappings exist:
const roleFeatureMappings = await featureRoleMappingQueries.findAll(filterQuery) if (roleFeatureMappings?.length > 0) { // Separate mappings and build role-feature maps in single pass const currentOrgRoleFeatureMap = new Map() const defaultOrgRoleFeatureMap = new Map() // Build maps of roles to features for current and default organizations roleFeatureMappings.forEach((mapping) => { const targetMap = mapping.organization_code === orgCode ? currentOrgRoleFeatureMap : defaultOrgRoleFeatureMap if (!targetMap.has(mapping.role_title)) { targetMap.set(mapping.role_title, new Set()) } targetMap.get(mapping.role_title).add(mapping.feature_code) }) // For each role, determine accessible features // If role has mappings in current org, use those; otherwise use default org mappings const accessibleFeatureCodes = new Set() roleTitles.forEach((role) => { const currentOrgFeatures = currentOrgRoleFeatureMap.get(role) if (currentOrgFeatures?.size > 0) { // Current org has explicit mappings for this role - use only those currentOrgFeatures.forEach((feature) => accessibleFeatureCodes.add(feature)) } else { // No current org mappings for this role - fall back to default org const defaultOrgFeatures = defaultOrgRoleFeatureMap.get(role) defaultOrgFeatures?.forEach((feature) => accessibleFeatureCodes.add(feature)) } }) // Filter to only accessible features organizationFeatures = organizationFeatures.filter((feature) => accessibleFeatureCodes.has(feature.feature_code) ) + } else { + // No role mappings found - restrict to empty set for security + console.warn(`No role mappings found for tenant: ${tenantCode}, org: ${orgCode}, roles: ${roleTitles.join(', ')}`) + organizationFeatures = [] } } }
203-209: Explicitly rollback transaction before early return.When role validation fails during update, the method returns without explicitly rolling back the transaction. Add an explicit rollback before returning the failure response.
Apply this diff:
if (validRoles.length !== bodyData.roles.length) { + await transaction.rollback() return responses.failureResponse({ message: 'ROLE_NOT_FOUND', statusCode: httpStatusCode.bad_request, responseCode: 'CLIENT_ERROR', }) }
🧹 Nitpick comments (1)
src/database/migrations/20251002165109-map-roles-to-features.js (1)
17-36: Consider documenting maintenance requirements for role-feature mappings.The hardcoded
roleMappingsobject defines the initial feature access for each role. When new roles or features are added in the future, this migration won't automatically update the mappings. Ensure your team has a process to:
- Update role-feature mappings when introducing new roles or features
- Document the expected feature set for each role
- Consider creating a configuration file or database-driven approach for ongoing maintenance
This is acceptable for initial setup but may require manual updates or additional migrations for future role/feature additions.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/database/migrations/20251002164809-create-feature-role-mapping-table.js(1 hunks)src/database/migrations/20251002165109-map-roles-to-features.js(1 hunks)src/services/organization-feature.js(9 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/services/**
⚙️ CodeRabbit configuration file
This is core business logic. Please check for correctness, efficiency, and potential edge cases.
Files:
src/services/organization-feature.js
🧬 Code graph analysis (2)
src/services/organization-feature.js (2)
src/services/tenant.js (2)
common(10-10)tokenInformation(847-851)src/controllers/v1/organization-feature.js (3)
common(10-10)tenantCode(69-69)orgCode(68-68)
src/database/migrations/20251002165109-map-roles-to-features.js (1)
src/services/organization-feature.js (2)
featureRoleMappingData(121-128)featureRoleMappingData(222-229)
🔇 Additional comments (2)
src/database/migrations/20251002164809-create-feature-role-mapping-table.js (1)
51-55: Verify the composite primary key design choice.The composite primary key on
(id, tenant_code)instead of justidsuggests a multi-tenant sharding/partitioning strategy where the sameidvalue can exist across different tenants. This is acceptable if intentional, but differs from typical single-column auto-increment primary keys.Please confirm this design aligns with your data partitioning strategy and that downstream queries consistently include
tenant_codein WHERE clauses to leverage partition pruning.src/database/migrations/20251002165109-map-roles-to-features.js (1)
70-118: LGTM! Well-structured mapping creation logic.The migration correctly:
- Validates roles exist in the tenant before creating mappings
- Ensures features are enabled for the organization before mapping
- Avoids duplicate mappings using a Set
- Logs skipped mappings for debugging
The use of
created_by: 0andupdated_by: 0is appropriate for system-generated migration data.
…nization features to empty set when no role mappings are found
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/services/organization-feature.js (1)
189-195: Rollback transaction before early return.When no rows are updated, the method returns early without rolling back the transaction, leaving it open until it times out or goes out of scope.
Apply this diff to explicitly rollback:
// Return error if no record was updated if (updatedCount === 0) { + await transaction.rollback() return responses.failureResponse({ message: 'FAILED_TO_UPDATE_ORG_FEATURE', statusCode: httpStatusCode.bad_request, responseCode: 'CLIENT_ERROR', }) }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/services/organization-feature.js(9 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/services/**
⚙️ CodeRabbit configuration file
This is core business logic. Please check for correctness, efficiency, and potential edge cases.
Files:
src/services/organization-feature.js
🧬 Code graph analysis (1)
src/services/organization-feature.js (2)
src/middlewares/authenticator.js (3)
common(11-11)tenantCode(220-220)orgCode(219-219)src/controllers/v1/organization-feature.js (3)
common(10-10)tenantCode(69-69)orgCode(68-68)
🔇 Additional comments (2)
src/services/organization-feature.js (2)
295-358: LGTM! Role-based filtering correctly implemented.The role-based filtering logic properly:
- Distinguishes admin roles from regular roles
- Prefers current organization mappings over default organization mappings for each role
- Returns an empty set when no mappings exist for non-admin users (addresses the security concern from past comments)
- Efficiently uses Maps and Sets for performance
The implementation correctly addresses the requirement to "give preference to users org role feature mapping" mentioned in past comments.
204-210: Explicitly rollback transaction before returning failure.The create method at line 114 explicitly rolls back the transaction when role validation fails, but this parallel validation in update does not. For consistency and clarity, add an explicit rollback here as well.
Apply this diff:
if (validRoles.length !== bodyData.roles.length) { + await transaction.rollback() return responses.failureResponse({ message: 'ROLE_NOT_FOUND', statusCode: httpStatusCode.bad_request, responseCode: 'CLIENT_ERROR', }) }Likely an incorrect or invalid review comment.
…ization features
Summary by CodeRabbit
New Features
Improvements