Skip to content
This repository was archived by the owner on Apr 6, 2022. It is now read-only.

Conversation

uxengine
Copy link
Contributor

@uxengine uxengine commented Mar 8, 2018

Kicking off backend changes for storing a reference to an audio version of an article. The current plan is to have input fields in an article's meta section for entering a URL of a previously uploaded mp3 and optionally aac and/or ogg. These may eventually turn into file upload fields.

Publikator PR: orbiting/publikator-frontend#140
Frontend PR: orbiting/republik-frontend#84

@uxengine uxengine requested a review from patte March 8, 2018 13:21
@uxengine uxengine changed the title Audio feat: audioSources Mar 8, 2018
@coveralls
Copy link

coveralls commented Mar 8, 2018

Pull Request Test Coverage Report for Build 45

  • 0 of 0 (NaN%) changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-4.7%) to 59.783%

Totals Coverage Status
Change from base Build 31: -4.7%
Covered Lines: 93
Relevant Lines: 136

💛 - Coveralls

format: resolver(meta.format),
discussion: resolver(meta.discussion)
discussion: resolver(meta.discussion),
audioSources: meta.audioSources
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is not needed

# the id of the discussion itself
discussionId: ID
credits: JSON
audioSources: [AudioSource]
Copy link
Contributor

Choose a reason for hiding this comment

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

This is going to be interesting... if the publikator writes something to audioSources array that doesn't fit the AudioSource spec, apollo is going to explode on delivery of the document. But I think this fine.

@uxengine
Copy link
Contributor Author

uxengine commented Mar 9, 2018

@patte I changed my mind. I'm storing flat properties (audioSourceMp3, audioSourceAac, audioSourceOgg) now in the document's meta, and resolve that to an object on the query:

audioSource {
  mp3
  aac
  ogg
}

audioSource
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

metaFieldResolver is only done on publish but audioSource should be available before. therefor and because what you do here is a simple sync transform, I recommend you put it in the main Meta resolver in: servers/publikator/graphql/resolvers/Meta.js.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry the right place is getMeta in packages/documents/lib/meta.js
This works everywhere (publikator- and republik-api).

@patte
Copy link
Contributor

patte commented Mar 12, 2018

lgtm, merge at will

@uxengine uxengine merged commit 6e1011b into master Mar 12, 2018
@uxengine uxengine deleted the audio branch March 12, 2018 16:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants