Skip to content

feat: feature config flag for import billable tooltip dismissal#933

Merged
JustARatherRidiculouslyLongUsername merged 1 commit intomasterfrom
import-billable-tooltip-dismissed
Mar 3, 2026
Merged

feat: feature config flag for import billable tooltip dismissal#933
JustARatherRidiculouslyLongUsername merged 1 commit intomasterfrom
import-billable-tooltip-dismissed

Conversation

@JustARatherRidiculouslyLongUsername
Copy link
Contributor

@JustARatherRidiculouslyLongUsername JustARatherRidiculouslyLongUsername commented Mar 3, 2026

@github-actions
Copy link

github-actions bot commented Mar 3, 2026

Coverage

Tests Skipped Failures Errors Time
845 0 💤 0 ❌ 0 🔥 1m 13s ⏱️

@github-actions
Copy link

github-actions bot commented Mar 3, 2026


Diff Coverage
Diff: origin/master..HEAD, staged and unstaged changes

apps/workspaces/models.py (100%)
apps/workspaces/serializers.py (100%)
apps/workspaces/views.py (100%)

Total: 28 lines
Missing: 0 lines
Coverage: 100%

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 3, 2026

📝 Walkthrough

Walkthrough

This pull request extends the workspace feature configuration capability by adding a new boolean field to track tooltip dismissal state for billable imports. The FeatureConfig model gains the import_billable_tooltip_dismissed field, the serializer restricts write access to specific fields via read-only declarations, and the API endpoint is upgraded from read-only to support both retrieval and updates via the RetrieveUpdateAPIView base class.

Poem

A tooltip dismissed, now tracked with care,
Read-only guards what should stay fair,
Updates now flow where reads once stood,
The config endpoint understood! 🎉

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is incomplete and lacks substantive details about the changes being made. Add a detailed description explaining what the feature flag does, why it's needed, and how it's implemented. The Description section must contain actual content beyond the template placeholder.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding a feature config flag for import billable tooltip dismissal, which is reflected in the new field added to FeatureConfig model.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/workspaces/models.py`:
- Line 273: The cache_key_map in FeatureConfig.get_feature_config is missing the
import_billable_tooltip_dismissed entry which causes AttributeError when
FeatureConfig.get_feature_config(workspace_id,
'import_billable_tooltip_dismissed') returns None and the code tries to access
.value; fix by adding the 'import_billable_tooltip_dismissed' mapping to
cache_key_map (the same pattern used for other boolean fields) or alternatively
make get_feature_config safe for unknown keys by returning a default
FeatureConfig-like object or checking for None before accessing .value in
FeatureConfig.get_feature_config.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 03ad147 and c5509d7.

⛔ Files ignored due to path filters (3)
  • apps/workspaces/migrations/0061_featureconfig_import_billable_tooltip_dismissed.py is excluded by !**/migrations/**
  • tests/sql_fixtures/reset_db_fixtures/reset_db.sql is excluded by !**/tests/**
  • tests/test_workspaces/test_views.py is excluded by !**/tests/**
📒 Files selected for processing (3)
  • apps/workspaces/models.py
  • apps/workspaces/serializers.py
  • apps/workspaces/views.py

fyle_webhook_sync_enabled = models.BooleanField(default=True, help_text='Enable fyle attribute webhook sync')
migrated_to_rest_api = models.BooleanField(default=False, help_text='Migrated to using rest api')
import_billable_field_for_projects = models.BooleanField(default=False, help_text='Import billable field for projects')
import_billable_tooltip_dismissed = models.BooleanField(default=False, help_text='Import billable tooltip dismissed')
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Check for get_feature_config usage and imports
rg -n "get_feature_config" apps/workspaces/models.py -C 5

Repository: fylein/fyle-intacct-api

Length of output: 757


🏁 Script executed:

# Check for import_billable_tooltip_dismissed references
rg -rn "import_billable_tooltip_dismissed" apps/

Repository: fylein/fyle-intacct-api

Length of output: 287


🏁 Script executed:

# Look for cache_key_map definition
rg -n "cache_key_map" apps/workspaces/models.py -C 5

Repository: fylein/fyle-intacct-api

Length of output: 1073


🏁 Script executed:

# Check for feature flag enums
rg -n "FEATURE_CONFIG|class.*Feature" apps/workspaces/models.py -C 2

Repository: fylein/fyle-intacct-api

Length of output: 647


🏁 Script executed:

# Check if import_billable_tooltip_dismissed is accessed via get_feature_config
rg -rn "get_feature_config.*import_billable" apps/ --type py

Repository: fylein/fyle-intacct-api

Length of output: 169


🏁 Script executed:

# Check all calls to get_feature_config to see what keys are used
rg -rn "get_feature_config" apps/ --type py -A 1 | grep -E "(get_feature_config|\.get_feature_config)" | head -20

Repository: fylein/fyle-intacct-api

Length of output: 49


🏁 Script executed:

# Search more broadly for how FeatureConfig fields are accessed
rg -rn "FeatureConfig\." apps/ --type py | grep -v "migration" | head -20

Repository: fylein/fyle-intacct-api

Length of output: 2549


🏁 Script executed:

# Check the actual field name in the model - was it 'n' or full name?
cat -n apps/workspaces/models.py | sed -n '270,295p'

Repository: fylein/fyle-intacct-api

Length of output: 1905


import_billable_tooltip_dismissed missing from cache_key_map in get_feature_config method.

If this field is accessed via FeatureConfig.get_feature_config(workspace_id, 'import_billable_tooltip_dismissed'), it will raise AttributeError at line 294 when trying to call .value on None. Add the missing cache key entry to cache_key_map (lines 286–291) or handle unknown keys safely.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/workspaces/models.py` at line 273, The cache_key_map in
FeatureConfig.get_feature_config is missing the
import_billable_tooltip_dismissed entry which causes AttributeError when
FeatureConfig.get_feature_config(workspace_id,
'import_billable_tooltip_dismissed') returns None and the code tries to access
.value; fix by adding the 'import_billable_tooltip_dismissed' mapping to
cache_key_map (the same pattern used for other boolean fields) or alternatively
make get_feature_config safe for unknown keys by returning a default
FeatureConfig-like object or checking for None before accessing .value in
FeatureConfig.get_feature_config.

@JustARatherRidiculouslyLongUsername JustARatherRidiculouslyLongUsername merged commit f47a7d2 into master Mar 3, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Medium PR

Development

Successfully merging this pull request may close these issues.

2 participants