Skip to content

feat: add is_enterprise_only attribute to apps#1840

Open
edward-ly wants to merge 9 commits into
masterfrom
feat/1785/enterprise-apps
Open

feat: add is_enterprise_only attribute to apps#1840
edward-ly wants to merge 9 commits into
masterfrom
feat/1785/enterprise-apps

Conversation

@edward-ly

@edward-ly edward-ly commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

Resolves #1785. In lieu of validating subscription keys, for now, a simple include_enterprise query parameter is added to the /api/v1/apps.json endpoint which controls whether or not enterprise-only apps are included in the list.

🤖 AI (if applicable)

  • The content of this PR was partly or fully generated using AI

@edward-ly
edward-ly force-pushed the feat/1785/enterprise-apps branch from 5f46a06 to 20bca73 Compare July 16, 2026 05:59

This comment was marked as resolved.

@edward-ly
edward-ly force-pushed the feat/1785/enterprise-apps branch 5 times, most recently from fa9df3d to b90ac55 Compare July 16, 2026 18:25
@edward-ly
edward-ly requested a review from miaulalala July 16, 2026 19:58

@julien-nc julien-nc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not familiar with the appstore code so I had a look and: Lgtm.
Then I asked Qwen:

  • urls.py:45 — appapi_apps.json uses apps_all_etag which now reads include_enterprise from the request. But AppApiAppsView doesn't filter by is_enterprise_only, so sending ?include_enterprise=true to that endpoint would compute an etag over a different dataset than what's actually returned. Either:
    • Give AppApiAppsView its own etag function that ignores the param, or
    • Also filter enterprise apps in AppApiAppsView if that makes sense for the app API.
  • caching.py:42-57 — apps_etag and apps_last_modified still use App.objects.all(). The AppView (per-version) doesn't filter enterprise-only apps either. If the intent is to hide enterprise apps from the public app list, shouldn't the per-version endpoint also exclude them? This is the endpoint Nextcloud server actually calls to get compatible apps.
  • The AppSerializer doesn't include is_enterprise_only, so API consumers can't tell which apps are enterprise-only. This might be intentional, but seems like an oversight — at minimum the Nextcloud server would need to know.
  • views.py:198 — When an existing app owner re-registers (e.g. to update their certificate), is_enterprise_only gets overwritten from the request body. This means the owner can silently toggle the flag at any time via re-registration, which contradicts the issue's intent that only admins should be able to change it post-registration. Consider only setting it on creation
  • The query param parsing is duplicated between AppsView._include_enterprise (views.py) and _include_enterprise (caching.py) with slightly different implementations. Extract to a shared utility.

Wdyt?

@edward-ly
edward-ly force-pushed the feat/1785/enterprise-apps branch from b90ac55 to 264313b Compare July 17, 2026 16:00
@edward-ly

Copy link
Copy Markdown
Collaborator Author

All bullet points should now be addressed, thanks!

Signed-off-by: Edward Ly <contact@edward.ly>
Signed-off-by: Edward Ly <contact@edward.ly>
…r form

Signed-off-by: Edward Ly <contact@edward.ly>
Assisted-by: OpenCode:GLM-5.2
Signed-off-by: Edward Ly <contact@edward.ly>
@edward-ly
edward-ly force-pushed the feat/1785/enterprise-apps branch from 264313b to 557a6da Compare July 17, 2026 16:01
Signed-off-by: Edward Ly <contact@edward.ly>
…son endpoints

Assisted-by: OpenCode:GLM-5.2
Signed-off-by: Edward Ly <contact@edward.ly>
…orm endpoint

Signed-off-by: Edward Ly <contact@edward.ly>
Assisted-by: OpenCode:GLM-5.2
Signed-off-by: Edward Ly <contact@edward.ly>
@edward-ly
edward-ly force-pushed the feat/1785/enterprise-apps branch from 557a6da to 2fe3d7c Compare July 17, 2026 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement "enterprise-only" app support

3 participants