Skip to content

Commit

Permalink
Fix part of oppia#10700: Refactor Object Factories for StateCard (opp…
Browse files Browse the repository at this point in the history
…ia#13028)

* Made changes in the files where createNewCard is used

* Updated static method for StateCard
* Updated all use cases to use the new static method

* Removed the StateObjectFactory from angular root files

* Removed all the imports statements for stateobject factory

* Removed import statements

* Removed the StateCardObjectFactoryClass
* Rename the files

* Lint fixes

* nit fix

* nit fix

* nit fix

Co-authored-by: Vojtěch Jelínek <[email protected]>

* nit fix

* Nit fix

* Fixed exploration-engine-spec file

* nit fix

* Refactor all required files

* lint fix

* updated file names from object factory to models

* nit lint fix

* nit fix

* nit fix --

* Fixed frontend test errors

* fixed order in check_frontend_test_coverage

* nit fix

* nit fix

Co-authored-by: Vojtěch Jelínek <[email protected]>
  • Loading branch information
srikanthkb and vojtechjelinek authored Jun 27, 2021
1 parent 0ba9c7b commit 4172bcc
Show file tree
Hide file tree
Showing 26 changed files with 196 additions and 160 deletions.
6 changes: 2 additions & 4 deletions core/templates/base-components/oppia-root.directive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,8 @@ angular.module('oppia').directive('oppiaRoot', [
'SchemaDefaultValueService',
'SchemaUndefinedLastElementService', 'SidebarStatusService',
'SiteAnalyticsService', 'SkillObjectFactory',
'SolutionObjectFactory',
'SpeechSynthesisChunkerService',
'StateCardObjectFactory', 'StateClassifierMappingService',
'StateInteractionStatsService',
'SolutionObjectFactory', 'SpeechSynthesisChunkerService',
'StateClassifierMappingService', 'StateInteractionStatsService',
'StateObjectFactory', 'StateTopAnswersStatsBackendApiService',
'StateTopAnswersStatsService', 'StatesObjectFactory',
'StoryContentsObjectFactory',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

import { ChangeDetectorRef, Component, OnDestroy, OnInit } from '@angular/core';
import { downgradeComponent } from '@angular/upgrade/static';
import { StateCard } from 'domain/state_card/StateCardObjectFactory';
import { StateCard } from 'domain/state_card/state-card.model';
import { ExplorationPlayerStateService } from 'pages/exploration-player-page/services/exploration-player-state.service';
import { HintAndSolutionModalService } from 'pages/exploration-player-page/services/hint-and-solution-modal.service';
import { HintsAndSolutionManagerService } from 'pages/exploration-player-page/services/hints-and-solution-manager.service';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,19 @@
// limitations under the License.

/**
* @fileoverview Tests for StateCardObjectFactory.
* @fileoverview Tests for state-card.model.ts.
*/

import { TestBed } from '@angular/core/testing';

import { AudioTranslationLanguageService } from
'pages/exploration-player-page/services/audio-translation-language.service';
import { CamelCaseToHyphensPipe } from
'filters/string-utility-filters/camel-case-to-hyphens.pipe';
import { InteractionObjectFactory } from
'domain/exploration/InteractionObjectFactory';
import { StateCardObjectFactory } from
'domain/state_card/StateCardObjectFactory';
import { StateCard } from
'domain/state_card/state-card.model';
import { SubtitledUnicode } from
'domain/exploration/SubtitledUnicodeObjectFactory';
import { WrittenTranslationsObjectFactory } from
Expand All @@ -33,20 +35,21 @@ import { Voiceover } from 'domain/exploration/voiceover.model';


describe('State card object factory', () => {
let stateCardObjectFactory = null;
let interactionObjectFactory = null;
let writtenTranslationsObjectFactory = null;
let audioTranslationLanguageService: AudioTranslationLanguageService;
let _sampleCard = null;

beforeEach(() => {
TestBed.configureTestingModule({
providers: [CamelCaseToHyphensPipe]
});

stateCardObjectFactory = TestBed.get(StateCardObjectFactory);
interactionObjectFactory = TestBed.get(InteractionObjectFactory);
writtenTranslationsObjectFactory = TestBed.get(
WrittenTranslationsObjectFactory);
audioTranslationLanguageService = TestBed.get(
AudioTranslationLanguageService);

let interactionDict = {
answer_groups: [],
Expand All @@ -73,7 +76,7 @@ describe('State card object factory', () => {
hints: [],
id: 'TextInput'
};
_sampleCard = stateCardObjectFactory.createNewCard(
_sampleCard = StateCard.createNewCard(
'State 1', '<p>Content</p>', '<interaction></interaction>',
interactionObjectFactory.createFromBackendDict(interactionDict),
RecordedVoiceovers.createFromBackendDict({
Expand All @@ -95,7 +98,7 @@ describe('State card object factory', () => {
}
}),
writtenTranslationsObjectFactory.createEmpty(),
'content');
'content', audioTranslationLanguageService);
});

it('should be able to get the various fields', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,10 @@
// limitations under the License.

/**
* @fileoverview Factory for creating new frontend instances of State
* @fileoverview Model class for creating new frontend instances of State
* card domain objects used in the exploration player.
*/

import { downgradeInjectable } from '@angular/upgrade/static';
import { Injectable } from '@angular/core';
import cloneDeep from 'lodash/cloneDeep';

import { AppConstants } from 'app.constants';
Expand Down Expand Up @@ -242,14 +240,6 @@ export class StateCard {
get contentId(): string {
return this._contentId;
}
}

@Injectable({
providedIn: 'root'
})
export class StateCardObjectFactory {
constructor(
private audioTranslationLanguageService: AudioTranslationLanguageService) {}

/**
* @param {string} stateName - The state name for the current card.
Expand All @@ -261,19 +251,18 @@ export class StateCardObjectFactory {
* the properties of the card's interaction.
* @param {RecordedVoiceovers} recordedVoiceovers
* @param {string} contentId
* @param {AudioTranslationLanguageService} audioTranslationLanguageService
*/
createNewCard(
static createNewCard(
stateName: string, contentHtml: string, interactionHtml: string,
interaction: Interaction, recordedVoiceovers: RecordedVoiceovers,
writtenTranslations: WrittenTranslations, contentId: string): StateCard {
writtenTranslations: WrittenTranslations, contentId: string,
audioTranslationLanguageService: AudioTranslationLanguageService
): StateCard {
return new StateCard(
stateName, contentHtml, interactionHtml,
cloneDeep(interaction), [],
recordedVoiceovers, writtenTranslations, contentId,
this.audioTranslationLanguageService);
audioTranslationLanguageService);
}
}

angular.module('oppia').factory(
'StateCardObjectFactory',
downgradeInjectable(StateCardObjectFactory));
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,16 @@ import { FormsModule } from '@angular/forms';
import { HttpClientTestingModule } from '@angular/common/http/testing';
import { PlayerTranscriptService } from
'pages/exploration-player-page/services/player-transcript.service';
import { StateCardObjectFactory } from 'domain/state_card/StateCardObjectFactory';
import { StateCard } from 'domain/state_card/state-card.model';
import { RecordedVoiceovers } from 'domain/exploration/recorded-voiceovers.model';
import { WrittenTranslationsObjectFactory } from 'domain/exploration/WrittenTranslationsObjectFactory';
import { SwitchContentLanguageRefreshRequiredModalComponent } from
// eslint-disable-next-line max-len
'pages/exploration-player-page/switch-content-language-refresh-required-modal.component';
import { ImagePreloaderService } from 'pages/exploration-player-page/services/image-preloader.service';
import { MockTranslatePipe } from 'tests/unit-test-utils';
import { AudioTranslationLanguageService} from
'pages/exploration-player-page/services/audio-translation-language.service';

class MockContentTranslationLanguageService {
getCurrentContentLanguageCode() {
Expand All @@ -58,9 +60,9 @@ describe('Content language selector component', () => {
let contentTranslationLanguageService: ContentTranslationLanguageService;
let fixture: ComponentFixture<ContentLanguageSelectorComponent>;
let playerTranscriptService: PlayerTranscriptService;
let stateCardObjectFactory: StateCardObjectFactory;
let writtenTranslationsObjectFactory: WrittenTranslationsObjectFactory;
let imagePreloaderService: ImagePreloaderService;
let audioTranslationLanguageService: AudioTranslationLanguageService;

beforeEach(async(() => {
TestBed.configureTestingModule({
Expand All @@ -86,10 +88,11 @@ describe('Content language selector component', () => {
contentTranslationLanguageService = TestBed.get(
ContentTranslationLanguageService);
playerTranscriptService = TestBed.get(PlayerTranscriptService);
stateCardObjectFactory = TestBed.get(StateCardObjectFactory);
writtenTranslationsObjectFactory = TestBed.get(
WrittenTranslationsObjectFactory);
imagePreloaderService = TestBed.get(ImagePreloaderService);
audioTranslationLanguageService = TestBed.get(
AudioTranslationLanguageService);
fixture = TestBed.createComponent(ContentLanguageSelectorComponent);
component = fixture.componentInstance;
fixture.detectChanges();
Expand All @@ -110,12 +113,12 @@ describe('Content language selector component', () => {
contentTranslationLanguageService,
'setCurrentContentLanguageCode');

const card = stateCardObjectFactory.createNewCard(
const card = StateCard.createNewCard(
'State 1', '<p>Content</p>', '<interaction></interaction>',
null,
RecordedVoiceovers.createEmpty(),
writtenTranslationsObjectFactory.createEmpty(),
'content');
'content', audioTranslationLanguageService);
spyOn(playerTranscriptService, 'getCard').and.returnValue(card);
spyOn(imagePreloaderService, 'restartImagePreloader');

Expand All @@ -132,12 +135,12 @@ describe('Content language selector component', () => {
contentTranslationLanguageService,
'setCurrentContentLanguageCode');

const card = stateCardObjectFactory.createNewCard(
const card = StateCard.createNewCard(
'State 1', '<p>Content</p>', '<interaction></interaction>',
null,
RecordedVoiceovers.createEmpty(),
writtenTranslationsObjectFactory.createEmpty(),
'content');
'content', audioTranslationLanguageService);
card.addInputResponsePair({
learnerInput: '',
oppiaResponse: '',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
*/

import { Subscription } from 'rxjs';
import { StateCard } from 'domain/state_card/state-card.model';

require(
'components/question-directives/question-player/services/' +
Expand Down Expand Up @@ -46,7 +47,7 @@ require('domain/exploration/editable-exploration-backend-api.service.ts');
require('domain/exploration/read-only-exploration-backend-api.service.ts');
require('domain/question/pretest-question-backend-api.service.ts');
require('domain/skill/ConceptCardObjectFactory.ts');
require('domain/state_card/StateCardObjectFactory.ts');
require('domain/state_card/state-card.model.ts');
require('domain/story_viewer/story-viewer-backend-api.service.ts');
require('domain/story_viewer/story-viewer-domain.constants.ajs.ts');
require('domain/topic_viewer/topic-viewer-domain.constants.ajs.ts');
Expand Down Expand Up @@ -100,6 +101,8 @@ require('services/stateful/focus-manager.service.ts');
require(
'pages/exploration-player-page/exploration-player-page.constants.ajs.ts');
require('pages/interaction-specs.constants.ajs.ts');
require(
'pages/exploration-player-page/services/audio-translation-language.service');
require(
'pages/exploration-player-page/services/' +
'content-translation-manager.service.ts');
Expand Down Expand Up @@ -349,8 +352,8 @@ angular.module('oppia').directive('conversationSkin', [
controller: [
'$http', '$location', '$rootScope', '$scope', '$timeout', '$translate',
'$window', 'AlertsService', 'AudioPlayerService',
'AutogeneratedAudioPlayerService', 'ConceptCardBackendApiService',
'ContentTranslationLanguageService',
'AudioTranslationLanguageService', 'AutogeneratedAudioPlayerService',
'ConceptCardBackendApiService', 'ContentTranslationLanguageService',
'ContentTranslationManagerService',
'ContextService', 'CurrentInteractionService',
'ExplorationEngineService', 'ExplorationPlayerStateService',
Expand All @@ -364,8 +367,8 @@ angular.module('oppia').directive('conversationSkin', [
'PlayerTranscriptService', 'QuestionPlayerEngineService',
'QuestionPlayerStateService', 'ReadOnlyCollectionBackendApiService',
'RefresherExplorationConfirmationModalService',
'SiteAnalyticsService', 'StateCardObjectFactory',
'StatsReportingService', 'StoryViewerBackendApiService', 'UrlService',
'SiteAnalyticsService', 'StatsReportingService',
'StoryViewerBackendApiService', 'UrlService',
'UserService', 'WindowDimensionsService',
'COMPONENT_NAME_FEEDBACK', 'CONTENT_FOCUS_LABEL_PREFIX',
'CONTINUE_BUTTON_FOCUS_LABEL', 'DEFAULT_TWITTER_SHARE_MESSAGE_EDITOR',
Expand All @@ -380,8 +383,8 @@ angular.module('oppia').directive('conversationSkin', [
function(
$http, $location, $rootScope, $scope, $timeout, $translate,
$window, AlertsService, AudioPlayerService,
AutogeneratedAudioPlayerService, ConceptCardBackendApiService,
ContentTranslationLanguageService,
AudioTranslationLanguageService, AutogeneratedAudioPlayerService,
ConceptCardBackendApiService, ContentTranslationLanguageService,
ContentTranslationManagerService,
ContextService, CurrentInteractionService,
ExplorationEngineService, ExplorationPlayerStateService,
Expand All @@ -395,8 +398,8 @@ angular.module('oppia').directive('conversationSkin', [
PlayerTranscriptService, QuestionPlayerEngineService,
QuestionPlayerStateService, ReadOnlyCollectionBackendApiService,
RefresherExplorationConfirmationModalService,
SiteAnalyticsService, StateCardObjectFactory,
StatsReportingService, StoryViewerBackendApiService, UrlService,
SiteAnalyticsService, StatsReportingService,
StoryViewerBackendApiService, UrlService,
UserService, WindowDimensionsService,
COMPONENT_NAME_FEEDBACK, CONTENT_FOCUS_LABEL_PREFIX,
CONTINUE_BUTTON_FOCUS_LABEL, DEFAULT_TWITTER_SHARE_MESSAGE_EDITOR,
Expand Down Expand Up @@ -1197,9 +1200,9 @@ angular.module('oppia').directive('conversationSkin', [
$scope.displayedCard.getStateName()) && $scope.conceptCard) {
ExplorationPlayerStateService.recordNewCardAdded();
_addNewCard(
StateCardObjectFactory.createNewCard(
StateCard.createNewCard(
null, $scope.conceptCard.getExplanation(), null, null, null,
null, null));
null, null, AudioTranslationLanguageService));
$rootScope.$applyAsync();
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { Component, EventEmitter, Input, Output, ViewChild } from '@angular/core
import { downgradeComponent } from '@angular/upgrade/static';
import { NgbPopover } from '@ng-bootstrap/ng-bootstrap';
import { AppConstants } from 'app.constants';
import { InputResponsePair } from 'domain/state_card/StateCardObjectFactory';
import { InputResponsePair } from 'domain/state_card/state-card.model';
import { InteractionSpecsConstants } from 'pages/interaction-specs.constants';
import { AudioPlayerService } from 'services/audio-player.service';
import { AutogeneratedAudioPlayerService } from 'services/autogenerated-audio-player.service';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { NgbActiveModal } from '@ng-bootstrap/ng-bootstrap';
import { AngularHtmlBindWrapperDirective } from 'components/angular-html-bind/angular-html-bind-wrapper.directive';
import { RecordedVoiceovers } from 'domain/exploration/recorded-voiceovers.model';
import { SubtitledHtml } from 'domain/exploration/subtitled-html.model';
import { StateCard } from 'domain/state_card/StateCardObjectFactory';
import { StateCard } from 'domain/state_card/state-card.model';
import { AudioPlayerService } from 'services/audio-player.service';
import { AutogeneratedAudioPlayerService } from 'services/autogenerated-audio-player.service';
import { AudioTranslationManagerService } from '../services/audio-translation-manager.service';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { Component } from '@angular/core';
import { NgbActiveModal } from '@ng-bootstrap/ng-bootstrap';
import { RecordedVoiceovers } from 'domain/exploration/recorded-voiceovers.model';
import { SubtitledHtml } from 'domain/exploration/subtitled-html.model';
import { StateCard } from 'domain/state_card/StateCardObjectFactory';
import { StateCard } from 'domain/state_card/state-card.model';
import { AudioPlayerService } from 'services/audio-player.service';
import { AutogeneratedAudioPlayerService } from 'services/autogenerated-audio-player.service';
import { AudioTranslationManagerService } from '../services/audio-translation-manager.service';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing';
import { NgbActiveModal } from '@ng-bootstrap/ng-bootstrap';
import { AngularHtmlBindWrapperDirective } from 'components/angular-html-bind/angular-html-bind-wrapper.directive';
import { RecordedVoiceovers } from 'domain/exploration/recorded-voiceovers.model';
import { StateCard } from 'domain/state_card/StateCardObjectFactory';
import { StateCard } from 'domain/state_card/state-card.model';
import { AudioPlayerService } from 'services/audio-player.service';
import { AutogeneratedAudioPlayerService } from 'services/autogenerated-audio-player.service';
import { AudioTranslationManagerService } from '../services/audio-translation-manager.service';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { NgbActiveModal } from '@ng-bootstrap/ng-bootstrap';
import { Interaction } from 'domain/exploration/InteractionObjectFactory';
import { RecordedVoiceovers } from 'domain/exploration/recorded-voiceovers.model';
import { ShortAnswerResponse, Solution } from 'domain/exploration/SolutionObjectFactory';
import { StateCard } from 'domain/state_card/StateCardObjectFactory';
import { StateCard } from 'domain/state_card/state-card.model';
import { AudioPlayerService } from 'services/audio-player.service';
import { AutogeneratedAudioPlayerService } from 'services/autogenerated-audio-player.service';
import { AudioTranslationManagerService } from '../services/audio-translation-manager.service';
Expand Down
Loading

0 comments on commit 4172bcc

Please sign in to comment.