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

Initial Launch Branch (Do not merge) #21

Open
wants to merge 22 commits into
base: launch_branch
Choose a base branch
from

Conversation

BenHarris4848
Copy link
Contributor

Overview

Prepare a branch to launch to the play store.

Changes Made

  • Remove magazines tab
  • suppress debrief and magazine notifications
  • hid magazine publishers from publications list

Test Coverage

Manual testing

Next Steps

Launch!

@@ -25,6 +25,7 @@ data class Publication(
val slug: String,
val shoutouts: Double,
val websiteURL: String,
val contentTypes: List<String>,
Copy link
Member

Choose a reason for hiding this comment

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

I think this would be better as a list of ENUMS than raw strings so it scales better from the source of truth;

should be

    val contentTypes: List<ContentType>,
    val websiteURL: String,
    val numArticles: Double,
    val mostRecentArticle: Article? = null,
    val socials: List<Social>
)

enum class ContentType {
    MAGAZINES, ARTICLES
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Implemented these changes on emily-content-type (so that they can eventually be merged into main), then incorporated those changes into this branch. This way we can have the enum change in main next semester, and keep the filtering only in this branch.

@@ -55,12 +55,14 @@ class ArticleRepository @Inject constructor(private val networkApi: NetworkApi)
rssURL = publication.rssURL,
slug = publication.slug,
shoutouts = publication.shoutouts,
contentTypes = publication.contentTypes,
Copy link
Member

Choose a reason for hiding this comment

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

Instead of contentTypes = publication.contentTypes, we can create the enums from the content type itself:

contentTypes = publication.contentTypes.map {
      ContentType.valueOf(it.uppercase())
  })

Copy link
Member

Choose a reason for hiding this comment

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

Ditto everywhere

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.

3 participants