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.

Sign up for GitHub

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

Jump to bottom

Rename OngoingStory to PromotedStory #2546

Closed
jcqli opened this issue Jan 22, 2021 · 15 comments · Fixed by #4704
Closed

Rename OngoingStory to PromotedStory #2546

jcqli opened this issue Jan 22, 2021 · 15 comments · Fixed by #4704
Assignees
@adhiamboperes
Labels
Impact: Low Low perceived user impact (e.g. edge cases). Priority: Nice-to-have This work item is nice to have for its milestone. Z-ibt Temporary label for Ben to keep track of issues he's triaged.

Comments

@jcqli
Copy link
Contributor

jcqli commented Jan 22, 2021

Since changing promoted stories to include recently played stories, older played stories, and suggest stories, we want to rename instances of OngoingStory to be called PromotedStory as per this review.

In RecentlyPlayedFragmentPresenter:

  • ongoingStoryListSummaryResultLiveData => promotedStoryListSummaryResultLiveData
  • fun subscribeToOngoingStoryList => fun subscribeToPromotedStoryList
    In OngoingStoryViewModel:
  • class name OngoingStoryViewModel => PromotedStoryViewModel (and also it's usages elsewhere)
@jcqli jcqli added Priority: Nice-to-have This work item is nice to have for its milestone. good first issue This item is good for new contributors to make their pull request. labels Jan 22, 2021
@jcqli jcqli mentioned this issue Jan 22, 2021
Merged
8 tasks
@priyanka0906
Copy link
Contributor

May I work on this?

@jcqli
Copy link
Contributor Author

jcqli commented Jan 26, 2021

As a note, when it comes time I'd recommend waiting until after #2320 is merged to merge this since this follows-up on some changes from that earlier PR.

@priyanka0906
Copy link
Contributor

As a note, when it comes time I'd recommend waiting until after #2320 is merged to merge this since this follows-up on some changes from that earlier PR.

Okay.

@priyanka0906
Copy link
Contributor

@FareesHussain @jcqli Till then can I look for any other issue?

@FareesHussain
Copy link
Contributor

Sure

@FareesHussain
Copy link
Contributor

@jcqli please update the title as [blocked on #2320]

@jcqli jcqli changed the title Rename OngoingStory to PromotedStory Rename OngoingStory to PromotedStory [Blocked on #2320] Jan 27, 2021
@veena14cs
Copy link
Contributor

@priyanka0906 since #2320 is merged, this is unblocked and you can work on this.

@rt4914 rt4914 changed the title Rename OngoingStory to PromotedStory [Blocked on #2320] Rename OngoingStory to PromotedStory Feb 16, 2021
@rt4914 rt4914 added this to the Backlog milestone Feb 16, 2021
@priyanka0906
Copy link
Contributor

@priyanka0906 since #2320 is merged, this is unblocked and you can work on this.

@jcqli all instances of ongoingStory has to be changed in promotedStory like ongoingStoryClickListener and ongoingListAdapter?

@jcqli
Copy link
Contributor Author

jcqli commented Feb 25, 2021

@jcqli all instances of ongoingStory has to be changed in promotedStory like ongoingStoryClickListener and ongoingListAdapter?

Yes, for clarity and consistency that would be best.

@priyanka0906
Copy link
Contributor

@jcqli all instances of ongoingStory has to be changed in promotedStory like ongoingStoryClickListener and ongoingListAdapter?

Yes, for clarity and consistency that would be best.

okay. Thankyou

@jcqli jcqli mentioned this issue Feb 25, 2021
Closed
8 tasks
@priyanka0906 priyanka0906 mentioned this issue Feb 27, 2021
Closed
8 tasks
@Rohit1173
Copy link
Contributor

May I work on this

@prayutsu
Copy link
Contributor

@Rohit1173 Assigning this to you.

Rohit1173 added a commit to Rohit1173/oppia-android that referenced this issue Jul 29, 2021
@Rohit1173
FIX oppia#2546 Rename OngoingStory to PromotedStory
af00dc0
This was referenced Jul 29, 2021
Closed
Closed
@pandayed
Copy link
Contributor

pandayed commented Feb 4, 2022

Can you please assign this to me? Thanks.

@rishidyno
Copy link
Contributor

Assigned this to @xpandeyed.

@pandayed pandayed mentioned this issue Feb 26, 2022
Closed
@rt4914 rt4914 removed the good first issue This item is good for new contributors to make their pull request. label May 25, 2022
@rt4914
Copy link
Contributor

rt4914 commented May 25, 2022

Removing it from good-first-issue because it actually requires a lot of context about the project to rename things correctly.

@Broppia Broppia added issue_type_infrastructure Impact: Low Low perceived user impact (e.g. edge cases). labels Jul 29, 2022
@BenHenning BenHenning added Z-ibt Temporary label for Ben to keep track of issues he's triaged. issue_user_developer labels Sep 15, 2022
@BenHenning BenHenning removed this from the Backlog milestone Sep 16, 2022
@adhiamboperes adhiamboperes self-assigned this Nov 1, 2022
@adhiamboperes adhiamboperes mentioned this issue Nov 9, 2022
Merged
13 tasks
BenHenning added a commit that referenced this issue Nov 21, 2022
@adhiamboperes @BenHenning
Fix #2546: Rename OngoingStory to PromotedStory (#4704)
3059cc7 
## Explanation

Fixes #2546 

Rename instances of ../recentlyplayed/OngoingStory to PromotedStory.

- [x] ongoingStoryListSummaryResultLiveData =>
promotedStoryListSummaryResultLiveData
- [x] fun subscribeToOngoingStoryList => fun
subscribeToPromotedStoryList in RecentlyPlayedFragmentPresenter
- [x] fun subscribeToOngoingStoryList => fun
subscribeToPromotedStoryList in RecentlyPlayedFragment
- [x] class name OngoingStoryViewModel => PromotedStoryViewModel
- [x] class name OngoingListAdapter => PromotedStoryListAdapter
- [x] private class OngoingStoryViewHolder => PromotedStoryViewHolder
- [x] interface name OngoingStoryClickListener =>
PromotedStoryClickListener

Note: `ongoing_story_card` is used to show stories in the "recently
played screen", under "Played within the last week and recommended
stories", shown when a user clicks "View all promoted stories" from the
home screen. I have renamed it to `recently_played_story_card.xml`.

`promoted_story_card.xml` is the layout file used to show promoted
stories on the home page.

I removed `PromotedStoryClickListener`, `PromotedStoryListAdapter` and
`PromotedStoryViewModel` from Kdoc excemption since the kdocs have been
added.

## Essential Checklist
- [x] The PR title and explanation each start with "Fix #bugnum: " (If
this PR fixes part of an issue, prefix the title with "Fix part of
#bugnum: ...".)
- [x] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [x] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

Co-authored-by: Ben Henning <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@adhiamboperes adhiamboperes

Labels
Impact: Low Low perceived user impact (e.g. edge cases). Priority: Nice-to-have This work item is nice to have for its milestone. Z-ibt Temporary label for Ben to keep track of issues he's triaged.
Projects
[Team] Core Learner and Mastery flows & UI Frontend - Android
Archived in project
Milestone
No milestone