Skip to content

Commit 6ea1d23

Browse files
author
Kanishka
committed
refactor: replace statusMessage with skipReason and skipDetail (Suggestion only)
- Add Suggestion.SKIP_REASONS enum - Add skipReason and skipDetail attributes to Suggestion schema with postgrestField mapping - Remove statusMessage from Opportunity (hold out of scope per review) - Update unit tests
1 parent f2d6250 commit 6ea1d23

File tree

5 files changed

+42
-21
lines changed

5 files changed

+42
-21
lines changed

packages/spacecat-shared-data-access/src/models/opportunity/opportunity.schema.js

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,6 @@ const schema = new SchemaBuilder(Opportunity, OpportunityCollection)
6666
.addAttribute('tags', {
6767
type: 'set',
6868
items: 'string',
69-
})
70-
.addAttribute('statusMessage', {
71-
type: 'string',
7269
});
7370

7471
export default schema.build();

packages/spacecat-shared-data-access/src/models/suggestion/suggestion.model.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,15 @@ class Suggestion extends BaseModel {
4646
CONFIG_UPDATE: 'CONFIG_UPDATE',
4747
};
4848

49+
/** Predefined categories for skip reason. Maps to PostgreSQL skip_reason. */
50+
static SKIP_REASONS = {
51+
ALREADY_IMPLEMENTED: 'already_implemented',
52+
INACCURATE_OR_INCOMPLETE: 'inaccurate_or_incomplete',
53+
TOO_RISKY: 'too_risky',
54+
NO_REASON: 'no_reason',
55+
OTHER: 'other',
56+
};
57+
4958
// Import schemas from external file for maintainability
5059
static FIELD_TRANSFORMERS = FIELD_TRANSFORMERS;
5160

packages/spacecat-shared-data-access/src/models/suggestion/suggestion.schema.js

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,14 @@ const schema = new SchemaBuilder(Suggestion, SuggestionCollection)
5050
required: true,
5151
default: Suggestion.STATUSES.NEW,
5252
})
53-
.addAttribute('statusMessage', {
53+
.addAttribute('skipReason', {
54+
type: Object.values(Suggestion.SKIP_REASONS),
55+
postgrestField: 'skip_reason',
56+
})
57+
.addAttribute('skipDetail', {
5458
type: 'string',
59+
postgrestField: 'skip_detail',
60+
validate: (value) => !value || (typeof value === 'string' && value.length <= 500),
5561
});
5662

5763
export default schema.build();

packages/spacecat-shared-data-access/test/unit/models/opportunity/opportunity.model.test.js

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -338,17 +338,6 @@ describe('OpportunityModel', () => {
338338
});
339339
});
340340

341-
describe('getStatusMessage and setStatusMessage', () => {
342-
it('returns undefined when no status message is set', () => {
343-
expect(instance.getStatusMessage()).to.be.undefined;
344-
});
345-
346-
it('sets and gets the status message', () => {
347-
instance.setStatusMessage('Not relevant for our use case');
348-
expect(instance.getStatusMessage()).to.equal('Not relevant for our use case');
349-
});
350-
});
351-
352341
describe('getOrigin and setOrigin', () => {
353342
it('returns the origin of the opportunity', () => {
354343
expect(instance.getOrigin()).to.equal('ESS_OPS');

packages/spacecat-shared-data-access/test/unit/models/suggestion/suggestion.model.test.js

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -133,14 +133,34 @@ describe('SuggestionModel', () => {
133133
});
134134
});
135135

136-
describe('getStatusMessage and setStatusMessage', () => {
137-
it('returns undefined when no status message is set', () => {
138-
expect(instance.getStatusMessage()).to.be.undefined;
136+
describe('SKIP_REASONS', () => {
137+
it('has SKIP_REASONS enum', () => {
138+
expect(Suggestion.SKIP_REASONS).to.be.an('object');
139+
expect(Suggestion.SKIP_REASONS.ALREADY_IMPLEMENTED).to.equal('already_implemented');
140+
expect(Suggestion.SKIP_REASONS.TOO_RISKY).to.equal('too_risky');
141+
expect(Suggestion.SKIP_REASONS.OTHER).to.equal('other');
142+
});
143+
});
144+
145+
describe('getSkipReason and setSkipReason', () => {
146+
it('returns undefined when no skip reason is set', () => {
147+
expect(instance.getSkipReason()).to.be.undefined;
148+
});
149+
150+
it('sets and gets the skip reason', () => {
151+
instance.setSkipReason('too_risky');
152+
expect(instance.getSkipReason()).to.equal('too_risky');
153+
});
154+
});
155+
156+
describe('getSkipDetail and setSkipDetail', () => {
157+
it('returns undefined when no skip detail is set', () => {
158+
expect(instance.getSkipDetail()).to.be.undefined;
139159
});
140160

141-
it('sets and gets the status message', () => {
142-
instance.setStatusMessage('Not relevant for our use case');
143-
expect(instance.getStatusMessage()).to.equal('Not relevant for our use case');
161+
it('sets and gets the skip detail', () => {
162+
instance.setSkipDetail('Not relevant for our use case');
163+
expect(instance.getSkipDetail()).to.equal('Not relevant for our use case');
144164
});
145165
});
146166

0 commit comments

Comments
 (0)