Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MMT-3409: publishDraft mutation for Collection/Service/Tool drafts, deleteTool mutation #73

Merged
merged 3 commits into from
Oct 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
344 changes: 273 additions & 71 deletions README.md

Large diffs are not rendered by default.

65 changes: 0 additions & 65 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 2 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -76,17 +76,16 @@
"jsonwebtoken": "^9.0.0",
"jwks-rsa": "^2.1.5",
"lodash": "^4.17.21",
"pascalcase-keys": "^1.0.1",
"qs": "^6.11.0",
"serverless": "^3.28.1",
"serverless-offline": "^12.0.4",
"serverless-plugin-log-subscription": "^2.2.0",
"serverless-python-requirements": "github:william-valencia/serverless-python-requirements#master",
"serverless-webpack": "^5.11.0",
"serverless": "^3.28.1",
"snakecase-keys": "^5.4.5",
"uuid": "^9.0.0",
"webpack-node-externals": "^3.0.0",
"webpack": "^5.76.1",
"webpack-node-externals": "^3.0.0",
"ws": "^8.12.1"
},
"overrides": {
Expand Down
66 changes: 50 additions & 16 deletions src/cmr/concepts/__tests__/draft.test.js
Original file line number Diff line number Diff line change
@@ -1,25 +1,39 @@
import Draft from '../draft'

describe('Draft concept', () => {
const OLD_ENV = process.env

beforeEach(() => {
jest.resetAllMocks()

jest.restoreAllMocks()

process.env = { ...OLD_ENV }

process.env.ummCollectionVersion = '1.0.0'
process.env.ummServiceVersion = '1.0.0'
process.env.ummToolVersion = '1.0.0'
process.env.ummVariableVersion = '1.0.0'
})

afterEach(() => {
process.env = OLD_ENV
})

describe('Draft concept', () => {
describe('constructor', () => {
describe('when the conceptType is collection-drafts', () => {
test('sets the ingestPath and metadataSpecification correctly', () => {
const draft = new Draft('collection-drafts', {}, {}, {
providerId: 'EDSC',
ummVersion: '1.0.0'
draftConceptId: 'CD100000-EDSC',
providerId: 'EDSC'
})

expect(draft.ingestPath).toEqual('providers/EDSC/collection-drafts')
expect(draft.publishPath).toEqual('publish/CD100000-EDSC')
expect(draft.metadataSpecification).toEqual({
Name: 'collection-draft',
URL: 'https://cdn.earthdata.nasa.gov/umm/collection-draft/v1.0.0',
Name: 'UMM-C',
URL: 'https://cdn.earthdata.nasa.gov/umm/collection/v1.0.0',
Version: '1.0.0'
})
})
Expand All @@ -28,14 +42,15 @@ describe('Draft concept', () => {
describe('when the conceptType is service-drafts', () => {
test('sets the ingestPath and metadataSpecification correctly', () => {
const draft = new Draft('service-drafts', {}, {}, {
providerId: 'EDSC',
ummVersion: '1.0.0'
draftConceptId: 'SD100000-EDSC',
providerId: 'EDSC'
})

expect(draft.ingestPath).toEqual('providers/EDSC/service-drafts')
expect(draft.publishPath).toEqual('publish/SD100000-EDSC')
expect(draft.metadataSpecification).toEqual({
Name: 'service-draft',
URL: 'https://cdn.earthdata.nasa.gov/umm/service-draft/v1.0.0',
Name: 'UMM-S',
URL: 'https://cdn.earthdata.nasa.gov/umm/service/v1.0.0',
Version: '1.0.0'
})
})
Expand All @@ -44,14 +59,15 @@ describe('Draft concept', () => {
describe('when the conceptType is tool-drafts', () => {
test('sets the ingestPath and metadataSpecification correctly', () => {
const draft = new Draft('tool-drafts', {}, {}, {
providerId: 'EDSC',
ummVersion: '1.0.0'
draftConceptId: 'TD100000-EDSC',
providerId: 'EDSC'
})

expect(draft.ingestPath).toEqual('providers/EDSC/tool-drafts')
expect(draft.publishPath).toEqual('publish/TD100000-EDSC')
expect(draft.metadataSpecification).toEqual({
Name: 'tool-draft',
URL: 'https://cdn.earthdata.nasa.gov/umm/tool-draft/v1.0.0',
Name: 'UMM-T',
URL: 'https://cdn.earthdata.nasa.gov/umm/tool/v1.0.0',
Version: '1.0.0'
})
})
Expand All @@ -60,18 +76,36 @@ describe('Draft concept', () => {
describe('when the conceptType is variable-drafts', () => {
test('sets the ingestPath and metadataSpecification correctly', () => {
const draft = new Draft('variable-drafts', {}, {}, {
providerId: 'EDSC',
ummVersion: '1.0.0'
draftConceptId: 'VD100000-EDSC',
providerId: 'EDSC'
})

expect(draft.ingestPath).toEqual('providers/EDSC/variable-drafts')
expect(draft.publishPath).toEqual('publish/VD100000-EDSC')
expect(draft.metadataSpecification).toEqual({
Name: 'variable-draft',
URL: 'https://cdn.earthdata.nasa.gov/umm/variable-draft/v1.0.0',
Name: 'UMM-Var',
URL: 'https://cdn.earthdata.nasa.gov/umm/variable/v1.0.0',
Version: '1.0.0'
})
})
})

describe('when the conceptType is not a supported draft', () => {
test('sets the ingestPath and metadataSpecification correctly', () => {
const draft = new Draft('bad-drafts', {}, {}, {
draftConceptId: 'CD100000-EDSC',
providerId: 'EDSC'
})

expect(draft.ingestPath).toEqual('providers/EDSC/bad-drafts')
expect(draft.publishPath).toEqual('publish/CD100000-EDSC')
expect(draft.metadataSpecification).toEqual({
Copy link
Member

Choose a reason for hiding this comment

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

Seems like this should throw an error instead of return bad data... Is the idea here that we send this to CMR and let them report the issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its really unreachable code. The conceptType in that switch statement is based off a graphql enum type, but I didn't want to write a ton of if statements so I wrote a switch. eslint expects a default case so this test handles the default. I can change it to throw an error, but it is still something we wouldn't see

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seeing all the tests I broke when I changed it to throw an error. When we call publishDraft it still uses this class, but conceptType is unimportant, so I just passed in Draft, which hits the default case in the switch. But the metadataSpecification isn't used when publishing a draft

Name: undefined,
URL: 'https://cdn.earthdata.nasa.gov/umm/undefined/vundefined',
Version: undefined
})
})
})
})
})
})
5 changes: 2 additions & 3 deletions src/cmr/concepts/collectionDraftProposal.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,7 @@ export default class CollectionDraftProposal extends Concept {
}

fetch(searchParams) {
// eslint-disable-next-line no-param-reassign
searchParams = mergeParams(searchParams)
const params = mergeParams(searchParams)

// Default an array to hold the promises we need to make depending on the requested fields
const promises = []
Expand All @@ -106,7 +105,7 @@ export default class CollectionDraftProposal extends Concept {

// Construct the promise that will request data from the umm endpoint
promises.push(
this.fetchUmm(searchParams, ummKeys, ummHeaders)
this.fetchUmm(params, ummKeys, ummHeaders)
)

this.response = Promise.all(promises)
Expand Down
22 changes: 9 additions & 13 deletions src/cmr/concepts/concept.js
Original file line number Diff line number Diff line change
Expand Up @@ -362,15 +362,14 @@ export default class Concept {
* @param {Object} providedHeaders Headers requested by the query
*/
ingest(data, requestedKeys, providedHeaders) {
// eslint-disable-next-line no-param-reassign
data = mergeParams(data)
const params = mergeParams(data)

this.logKeyRequest(requestedKeys, 'ingest')

// Construct the promise that will ingest data into CMR
this.response = cmrIngest(
this.getConceptType(),
data,
params,
providedHeaders,
this.ingestPath
)
Expand All @@ -383,15 +382,14 @@ export default class Concept {
* @param {Object} providedHeaders Headers requested by the query
*/
delete(data, requestedKeys, providedHeaders) {
// eslint-disable-next-line no-param-reassign
data = mergeParams(data)
const params = mergeParams(data)

this.logKeyRequest(requestedKeys, 'ingest')

// Construct the promise that will delete data from CMR
this.response = cmrDelete(
this.getConceptType(),
data,
params,
providedHeaders,
this.ingestPath
)
Expand Down Expand Up @@ -455,8 +453,7 @@ export default class Concept {
* @param {Object} searchParams Parameters provided by the query
*/
fetch(searchParams) {
// eslint-disable-next-line no-param-reassign
searchParams = mergeParams(searchParams)
const params = mergeParams(searchParams)

// Default an array to hold the promises we need to make depending on the requested fields
const promises = []
Expand All @@ -469,11 +466,10 @@ export default class Concept {

const {
cursor
} = searchParams
} = params

if (cursor) {
// eslint-disable-next-line no-param-reassign
delete searchParams.cursor
delete params.cursor
}

const {
Expand All @@ -493,7 +489,7 @@ export default class Concept {
}

promises.push(
this.fetchJson(this.arrayifyParams(searchParams), jsonKeys, jsonHeaders)
this.fetchJson(this.arrayifyParams(params), jsonKeys, jsonHeaders)
)
} else {
// Push a null promise to the array so that the umm promise always exists as
Expand All @@ -516,7 +512,7 @@ export default class Concept {

// Construct the promise that will request data from the umm endpoint
promises.push(
this.fetchUmm(this.arrayifyParams(searchParams), ummKeys, ummHeaders)
this.fetchUmm(this.arrayifyParams(params), ummKeys, ummHeaders)
)
} else {
promises.push(
Expand Down
Loading