fix(twenty-server): re-list marketplace registration when catalog serves an app first installed locally (#22877)
## Problem Fixes #22872. An app first installed from a **local/CLI source** (`yarn twenty dev` / tarball upload) gets its `applicationRegistration` created with `isListed: false` — sensible for a dev app. But when that same app (same `universalIdentifier`) is later **published to the configured app registry**, the marketplace catalog sync's update branch in `upsertFromCatalog` spreads the existing entity and updates name/sourceType/sourcePackage/version/manifest **without ever setting `isListed` back to `true`** (only the create branch does). Result: the app is permanently invisible in the Marketplace tab (`findManyMarketplaceApps` → `findManyListed()`), while `installApplication(universalIdentifier)` still works — a confusing split-brain state with no error anywhere. Reproduced on a self-hosted v2.18.5 with a private Verdaccio registry (details and repro steps in the issue). ## Fix In the `upsertFromCatalog` update branch, re-list the registration **only when its previous source was local** (`TARBALL`/`LOCAL`): ```ts const isRelistedFromLocalSource = existing.sourceType === ApplicationRegistrationSourceType.TARBALL || existing.sourceType === ApplicationRegistrationSourceType.LOCAL; ... isListed: existing.isListed || isRelistedFromLocalSource, ``` This deliberately does **not** blanket-set `isListed: true` on every sync: an operator who delisted a registry-sourced app (via `updateApplicationRegistration`) keeps their decision — the hourly sync won't override it. ## Tests New unit spec `application-registration-upsert-from-catalog.spec.ts`: - re-lists a registration first created by a local install once the catalog serves it; - preserves an operator delisting of a registry-sourced registration; - keeps an already-listed registration listed; - still creates new catalog registrations as listed. <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22877?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> Co-authored-by: Nicolas Chanal <nicolaschanal@MacBook-Pro-de-Nicolas.local> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
+154
@@ -0,0 +1,154 @@
|
||||
import { Test, type TestingModule } from '@nestjs/testing';
|
||||
import { getRepositoryToken } from '@nestjs/typeorm';
|
||||
|
||||
import { ApplicationRegistrationAssetUrlService } from 'src/engine/core-modules/application/application-registration/application-registration-asset-url.service';
|
||||
import { ApplicationRegistrationEntity } from 'src/engine/core-modules/application/application-registration/application-registration.entity';
|
||||
import { ApplicationRegistrationService } from 'src/engine/core-modules/application/application-registration/application-registration.service';
|
||||
import { ApplicationRegistrationVariableService } from 'src/engine/core-modules/application/application-registration-variable/application-registration-variable.service';
|
||||
import { ApplicationRegistrationSourceType } from 'src/engine/core-modules/application/application-registration/enums/application-registration-source-type.enum';
|
||||
import { ApplicationEntity } from 'src/engine/core-modules/application/application.entity';
|
||||
import { CacheLockService } from 'src/engine/core-modules/cache-lock/cache-lock.service';
|
||||
import { CoreEntityCacheService } from 'src/engine/core-entity-cache/services/core-entity-cache.service';
|
||||
import { ServerFileStorageService } from 'src/engine/core-modules/file-storage/services/server-file-storage.service';
|
||||
import { WorkspaceEntity } from 'src/engine/core-modules/workspace/workspace.entity';
|
||||
|
||||
describe('ApplicationRegistrationService - upsertFromCatalog', () => {
|
||||
let service: ApplicationRegistrationService;
|
||||
let applicationRegistrationRepository: {
|
||||
findOne: jest.Mock;
|
||||
save: jest.Mock;
|
||||
create: jest.Mock;
|
||||
};
|
||||
|
||||
const catalogParams = {
|
||||
universalIdentifier: '97141c95-2870-5662-8992-44fb6536be9a',
|
||||
name: 'My App',
|
||||
sourceType: ApplicationRegistrationSourceType.NPM,
|
||||
sourcePackage: 'twenty-app-my-app',
|
||||
latestAvailableVersion: '0.2.0',
|
||||
manifest: null,
|
||||
};
|
||||
|
||||
const buildExistingRegistration = (
|
||||
overrides: Partial<ApplicationRegistrationEntity>,
|
||||
) =>
|
||||
({
|
||||
id: 'registration-id',
|
||||
universalIdentifier: catalogParams.universalIdentifier,
|
||||
name: 'My App',
|
||||
galleryImages: [],
|
||||
...overrides,
|
||||
}) as ApplicationRegistrationEntity;
|
||||
|
||||
beforeEach(async () => {
|
||||
applicationRegistrationRepository = {
|
||||
findOne: jest.fn(),
|
||||
save: jest.fn(),
|
||||
create: jest.fn((entity) => entity),
|
||||
};
|
||||
|
||||
const module: TestingModule = await Test.createTestingModule({
|
||||
providers: [
|
||||
ApplicationRegistrationService,
|
||||
{
|
||||
provide: getRepositoryToken(ApplicationRegistrationEntity),
|
||||
useValue: applicationRegistrationRepository,
|
||||
},
|
||||
{
|
||||
provide: getRepositoryToken(ApplicationEntity),
|
||||
useValue: { find: jest.fn(), findOne: jest.fn() },
|
||||
},
|
||||
{
|
||||
provide: getRepositoryToken(WorkspaceEntity),
|
||||
useValue: { find: jest.fn(), findOne: jest.fn() },
|
||||
},
|
||||
{
|
||||
provide: ApplicationRegistrationVariableService,
|
||||
useValue: { syncVariableSchemas: jest.fn() },
|
||||
},
|
||||
{
|
||||
provide: ApplicationRegistrationAssetUrlService,
|
||||
useValue: { resolveAssetUrls: jest.fn() },
|
||||
},
|
||||
{
|
||||
provide: ServerFileStorageService,
|
||||
useValue: { write: jest.fn(), delete: jest.fn() },
|
||||
},
|
||||
{
|
||||
provide: CacheLockService,
|
||||
useValue: { withLock: jest.fn((_key, fn) => fn()) },
|
||||
},
|
||||
{
|
||||
provide: CoreEntityCacheService,
|
||||
useValue: { invalidate: jest.fn() },
|
||||
},
|
||||
],
|
||||
}).compile();
|
||||
|
||||
service = module.get<ApplicationRegistrationService>(
|
||||
ApplicationRegistrationService,
|
||||
);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
jest.clearAllMocks();
|
||||
});
|
||||
|
||||
it('should re-list a registration first created by a local install when the catalog serves it', async () => {
|
||||
applicationRegistrationRepository.findOne.mockResolvedValue(
|
||||
buildExistingRegistration({
|
||||
sourceType: ApplicationRegistrationSourceType.TARBALL,
|
||||
isListed: false,
|
||||
}),
|
||||
);
|
||||
|
||||
await service.upsertFromCatalog(catalogParams);
|
||||
|
||||
expect(applicationRegistrationRepository.save).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
isListed: true,
|
||||
sourceType: ApplicationRegistrationSourceType.NPM,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should preserve an operator delisting of a registry-sourced registration', async () => {
|
||||
applicationRegistrationRepository.findOne.mockResolvedValue(
|
||||
buildExistingRegistration({
|
||||
sourceType: ApplicationRegistrationSourceType.NPM,
|
||||
isListed: false,
|
||||
}),
|
||||
);
|
||||
|
||||
await service.upsertFromCatalog(catalogParams);
|
||||
|
||||
expect(applicationRegistrationRepository.save).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ isListed: false }),
|
||||
);
|
||||
});
|
||||
|
||||
it('should keep an already listed registration listed', async () => {
|
||||
applicationRegistrationRepository.findOne.mockResolvedValue(
|
||||
buildExistingRegistration({
|
||||
sourceType: ApplicationRegistrationSourceType.NPM,
|
||||
isListed: true,
|
||||
}),
|
||||
);
|
||||
|
||||
await service.upsertFromCatalog(catalogParams);
|
||||
|
||||
expect(applicationRegistrationRepository.save).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ isListed: true }),
|
||||
);
|
||||
});
|
||||
|
||||
it('should create new catalog registrations as listed', async () => {
|
||||
applicationRegistrationRepository.findOne.mockResolvedValue(null);
|
||||
|
||||
await service.upsertFromCatalog(catalogParams);
|
||||
|
||||
expect(applicationRegistrationRepository.save).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ isListed: true }),
|
||||
);
|
||||
});
|
||||
});
|
||||
+9
@@ -629,12 +629,21 @@ export class ApplicationRegistrationService {
|
||||
params.latestAvailableVersion ?? null,
|
||||
);
|
||||
|
||||
// A registration first created by a local install (CLI dev / tarball
|
||||
// upload) starts unlisted on purpose. Once the catalog source serves the
|
||||
// same universalIdentifier, surface it in the marketplace — while
|
||||
// preserving an operator's decision to delist a registry-sourced app.
|
||||
const isRelistedFromLocalSource =
|
||||
existing.sourceType === ApplicationRegistrationSourceType.TARBALL ||
|
||||
existing.sourceType === ApplicationRegistrationSourceType.LOCAL;
|
||||
|
||||
await this.applicationRegistrationRepository.save({
|
||||
...existing,
|
||||
name: params.name,
|
||||
sourceType: params.sourceType,
|
||||
sourcePackage: params.sourcePackage,
|
||||
latestAvailableVersion: params.latestAvailableVersion,
|
||||
isListed: existing.isListed || isRelistedFromLocalSource,
|
||||
isVetted,
|
||||
manifest: params.manifest,
|
||||
...fromManifestApplicationToDisplayFields(params.manifest?.application),
|
||||
|
||||
Reference in New Issue
Block a user