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

Upgrade more services to Angular 8 #7145

Merged
merged 38 commits into from
Jul 19, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
13424bf
Untested work on PlaythroughIssueObjectFactory.ts
YashJipkate Jul 8, 2019
5c8bec3
Revert "Untested work on PlaythroughIssueObjectFactory.ts"
YashJipkate Jul 8, 2019
3b51282
Merge branch 'develop' into ser-up-more
YashJipkate Jul 8, 2019
3ae97e8
TopicRightsObjectFactory
YashJipkate Jul 8, 2019
7f22747
Lint fix
YashJipkate Jul 8, 2019
ffa8a7e
Removed unnecessary constructor statement
YashJipkate Jul 8, 2019
952d5c0
ClassifierObjectFactory
YashJipkate Jul 8, 2019
41987d5
AnswerClassificationResultObjectFactory
YashJipkate Jul 9, 2019
9990fe5
PredictionResultObjectFactory
YashJipkate Jul 9, 2019
027340d
WrittenTranslationObjectFactory
YashJipkate Jul 9, 2019
cc0ec34
RuleObjectFactory
YashJipkate Jul 9, 2019
906fd76
AudioTranslationObjectFactory
YashJipkate Jul 9, 2019
5b56005
ExplorationDraftObjectFactory
YashJipkate Jul 9, 2019
1502ad2
Fix failing FE in TopicRightsObjectFactory
YashJipkate Jul 10, 2019
f3a0997
Merge branch 'develop' into ser-up-more
YashJipkate Jul 10, 2019
5e96b0f
Add force bootstrap to all pages
YashJipkate Jul 10, 2019
6f906ef
function() -> () =>
YashJipkate Jul 10, 2019
feb8a4c
Merge
YashJipkate Jul 11, 2019
56f61f0
Merge proper
YashJipkate Jul 11, 2019
64f9fea
Merge branch 'develop' into ser-up-more
YashJipkate Jul 14, 2019
69681da
Update mocking method
YashJipkate Jul 14, 2019
04d7091
Merge branch 'develop' into ser-up-more
YashJipkate Jul 15, 2019
727a6bf
More typing
YashJipkate Jul 16, 2019
abb470f
Audit and newline
YashJipkate Jul 16, 2019
d8e573b
Merge branch 'develop' into ser-up-more
YashJipkate Jul 16, 2019
6fa6c50
Merge develop
YashJipkate Jul 16, 2019
0ca2eae
Merge branch 'develop' into ser-up-more
YashJipkate Jul 17, 2019
5c75b38
Stronger typing
YashJipkate Jul 17, 2019
0e557bf
Fix FE
YashJipkate Jul 17, 2019
dff56fd
Address comments
YashJipkate Jul 17, 2019
e26e464
Remove newlines, TODO for imports, modify MockOutcome
YashJipkate Jul 18, 2019
aa5133d
Fix matcher case, modify TODO for imports
YashJipkate Jul 18, 2019
951982b
Stronger typing in RuleObjectFactory
YashJipkate Jul 18, 2019
de7d2ad
Fix e2e
YashJipkate Jul 19, 2019
ce5cec8
Fix AJAX errors in Karma
YashJipkate Jul 19, 2019
b5c2ef9
Merge develop
YashJipkate Jul 19, 2019
b6f243f
Fix FE
YashJipkate Jul 19, 2019
3df1a3d
Add comment in csrftoken
YashJipkate Jul 19, 2019
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
Prev Previous commit
Next Next commit
Fix matcher case, modify TODO for imports
  • Loading branch information
YashJipkate committed Jul 18, 2019
commit aa5133d3df1db3f5556586c7893d4e541b3729de
5 changes: 3 additions & 2 deletions core/templates/dev/head/AppSpec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,12 @@
* @fileoverview Unit tests for generic services.
*/

// TODO(YashJipkate) Remove the unnnecessary imports once the corresponding
// non-spec file is upgraded.
// TODO(YashJipkate) Remove the following block of unnnecessary imports once
// App.ts is upgraded to Angular 8.
import { RuleObjectFactory } from 'domain/exploration/RuleObjectFactory.ts';
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure I understand why these need to be added here. They don't seem to be used in the test?

There should probably be documentation somewhere to explain -- either here if it's specific to this, or in our wiki if it's a general principle to follow. In any case I think it would be nice to add a comment above line 25 explaining why lines 26-29 are needed. Do the same in all other similar files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not specific to this file, this is the case in many files. What's happening is that since we do not use isolated tests, these spec files depended upon the upgraded services and therefore angular mock could not interpret them.
This would be more suited to the wiki than a comment. I'll add this to the Angular wiki.

Copy link
Member

Choose a reason for hiding this comment

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

Is this something that would be removed after the full migration to Angular 8?

If so, we should add a TODO to remove them in the future, like Vojta did for some of the webpack requires. I realize that that might be repetitive, but we should do that anyway to make clear that this isn't a long-term practice to be followed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The statements can be removed once the corresponding non-spec file is upgraded.
Added the TODOs!

import { WrittenTranslationObjectFactory } from
'domain/exploration/WrittenTranslationObjectFactory.ts';
// ^^^ This block is to be removed.

describe('Constants Generating', function() {
beforeEach(angular.mock.module('oppia'));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,12 @@
* @fileoverview Unit tests for the question player state service.
*/

// TODO(YashJipkate) Remove the unnnecessary imports once the corresponding
// non-spec file is upgraded.
// TODO(YashJipkate) Remove the following block of unnnecessary imports once
// question-player-state.service.ts is upgraded to Angular 8.
import { RuleObjectFactory } from 'domain/exploration/RuleObjectFactory.ts';
import { WrittenTranslationObjectFactory } from
'domain/exploration/WrittenTranslationObjectFactory.ts';
// ^^^ This block is to be removed.

require(
'components/question-directives/question-player/services/' +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,11 @@
* editor page.
*/

// TODO(YashJipkate) Remove the unnnecessary imports once the corresponding
// non-spec file is upgraded.
// TODO(YashJipkate) Remove the following block of unnnecessary imports once
// state-property.service.ts is upgraded to Angular 8.
import { ExplorationDraftObjectFactory } from
'domain/exploration/ExplorationDraftObjectFactory.ts';
// ^^^ This block is to be removed.

require('pages/exploration-editor-page/services/change-list.service.ts');
require('pages/exploration-editor-page/services/exploration-title.service.ts');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,11 @@
* @fileoverview Unit tests for the controller of 'State Interactions'.
*/

// TODO(YashJipkate) Remove the unnnecessary imports once the corresponding
// non-spec file is upgraded.
// TODO(YashJipkate) Remove the following block of unnnecessary imports once
// state-interaction-editor.directive.ts is upgraded to Angular 8.
import { ExplorationFeaturesService } from
'services/ExplorationFeaturesService.ts';
// ^^^ This block is to be removed.

require(
'pages/exploration-editor-page/editor-tab/' +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,12 @@
* @fileoverview Unit tests for the Exploration object factory.
*/

// TODO(YashJipkate) Remove the unnnecessary imports once the corresponding
// non-spec file is upgraded.
// TODO(YashJipkate) Remove the following block of unnnecessary imports once
// ExplorationObjectFactory.ts is upgraded to Angular 8.
import { RuleObjectFactory } from 'domain/exploration/RuleObjectFactory.ts';
import { WrittenTranslationObjectFactory } from
'domain/exploration/WrittenTranslationObjectFactory.ts';
// ^^^ This block is to be removed.

require('domain/exploration/ExplorationObjectFactory.ts');
require('domain/exploration/VoiceoverObjectFactory.ts');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,10 @@
* @fileoverview Unit tests for Interaction object factory.
*/

// TODO(YashJipkate) Remove the unnnecessary imports once the corresponding
// non-spec file is upgraded.
// TODO(YashJipkate) Remove the following block of unnnecessary imports once
// InteractionObjectFactory.ts is upgraded to Angular 8.
import { RuleObjectFactory } from 'domain/exploration/RuleObjectFactory.ts';
// ^^^ This block is to be removed.

require('domain/exploration/AnswerGroupObjectFactory.ts');
require('domain/exploration/HintObjectFactory.ts');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,12 @@
* @fileoverview Unit tests for the States object factory.
*/

// TODO(YashJipkate) Remove the unnnecessary imports once the corresponding
// non-spec file is upgraded.
// TODO(YashJipkate) Remove the following block of unnnecessary imports once
// StatesObjectFactory.ts is upgraded to Angular 8.
import { RuleObjectFactory } from 'domain/exploration/RuleObjectFactory.ts';
import { WrittenTranslationObjectFactory } from
'domain/exploration/WrittenTranslationObjectFactory.ts';
// ^^^ This block is to be removed.

require('domain/exploration/StatesObjectFactory.ts');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,12 @@
* @fileoverview Tests for QuestionContentsObjectFactory.
*/

// TODO(YashJipkate) Remove the unnnecessary imports once the corresponding
// non-spec file is upgraded.
// TODO(YashJipkate) Remove the following block of unnnecessary imports once
// QuestionObjectFactory.ts is upgraded to Angular 8.
import { RuleObjectFactory } from 'domain/exploration/RuleObjectFactory.ts';
import { WrittenTranslationObjectFactory } from
'domain/exploration/WrittenTranslationObjectFactory.ts';
// ^^^ This block is to be removed.

require('domain/question/QuestionObjectFactory.ts');
require('domain/skill/MisconceptionObjectFactory.ts');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,12 @@
* @fileoverview Unit tests for question update service.
*/

// TODO(YashJipkate) Remove the unnnecessary imports once the corresponding
// non-spec file is upgraded.
// TODO(YashJipkate) Remove the following block of unnnecessary imports once
// QuestionUpdateService.ts is upgraded to Angular 8.
import { RuleObjectFactory } from 'domain/exploration/RuleObjectFactory.ts';
import { WrittenTranslationObjectFactory } from
'domain/exploration/WrittenTranslationObjectFactory.ts';
// ^^^ This block is to be removed.

require('App.ts');
require('domain/editor/undo_redo/QuestionUndoRedoService.ts');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,10 @@
* @fileoverview Tests for StateCardObjectFactory.
*/

// TODO(YashJipkate) Remove the unnnecessary imports once the corresponding
// non-spec file is upgraded.
// TODO(YashJipkate) Remove the following block of unnnecessary imports once
// StateCardObjectFactory.ts is upgraded to Angular 8.
import { RuleObjectFactory } from 'domain/exploration/RuleObjectFactory.ts';
// ^^^ This block is to be removed.

require('domain/exploration/InteractionObjectFactory.ts');
require('domain/exploration/RecordedVoiceoversObjectFactory.ts');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
* @fileoverview Unit tests for the FeedbackImprovementCardObjectFactory.
*/

// TODO(YashJipkate) Remove the unnnecessary imports once the corresponding
// non-spec file is upgraded.
// TODO(YashJipkate) Remove the following block of unnnecessary imports once
// FeedbackImprovementCardObjectFactory.ts is upgraded to Angular 8.
import { AnswerClassificationResultObjectFactory } from
'domain/classifier/AnswerClassificationResultObjectFactory.ts';
import { ExplorationDraftObjectFactory } from
Expand All @@ -27,6 +27,7 @@ import { ClassifierObjectFactory } from
import { RuleObjectFactory } from 'domain/exploration/RuleObjectFactory.ts';
import { WrittenTranslationObjectFactory } from
'domain/exploration/WrittenTranslationObjectFactory.ts';
// ^^^ This block is to be removed.

require('domain/statistics/FeedbackImprovementCardObjectFactory.ts');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
* @fileoverview Unit tests for the SuggestionImprovementCardObjectFactory.
*/

// TODO(YashJipkate) Remove the unnnecessary imports once the corresponding
// non-spec file is upgraded.
// TODO(YashJipkate) Remove the following block of unnnecessary imports once
// SuggestionImprovementCardObjectFactory.ts is upgraded to Angular 8.
import { AnswerClassificationResultObjectFactory } from
'domain/classifier/AnswerClassificationResultObjectFactory.ts';
import { ExplorationDraftObjectFactory } from
Expand All @@ -27,6 +27,7 @@ import { ClassifierObjectFactory } from
import { RuleObjectFactory } from 'domain/exploration/RuleObjectFactory.ts';
import { WrittenTranslationObjectFactory } from
'domain/exploration/WrittenTranslationObjectFactory.ts';
// ^^^ This block is to be removed.

require('domain/statistics/SuggestionImprovementCardObjectFactory.ts');

Expand Down
26 changes: 18 additions & 8 deletions core/templates/dev/head/domain/topic/TopicRightsObjectFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,20 +38,20 @@ export class TopicRights {
isPublished() {
return this._published;
}
canPublishTopic = function() {
canPublishTopic() {
return this._canPublishTopic;
}
canEditName = function() {
canEditName() {
return this._canPublishTopic;
}
markTopicAsPublished = function() {
markTopicAsPublished() {
if (this._canPublishTopic) {
this._published = true;
} else {
throw new Error('User is not allowed to publish this topic.');
}
}
markTopicAsUnpublished = function() {
markTopicAsUnpublished() {
if (this._canPublishTopic) {
this._published = false;
} else {
Expand All @@ -61,7 +61,12 @@ export class TopicRights {
// Reassigns all values within this topic to match the existing
// topic rights. This is performed as a deep copy such that none of the
// internal, bindable objects are changed within this topic rights.
copyFromTopicRights = function(otherTopicRights) {
copyFromTopicRights(
otherTopicRights: {
isPublished: () => boolean;
canEditTopic: () => boolean;
canPublishTopic: () => boolean;
}) {
this._published = otherTopicRights.isPublished();
this._canEditTopic = otherTopicRights.canEditTopic();
this._canPublishTopic = otherTopicRights.canPublishTopic();
Expand All @@ -74,11 +79,16 @@ export class TopicRights {
export class TopicRightsObjectFactory {
// This function takes a JSON object which represents a backend
// topic python dict.
createFromBackendDict(topicRightsBackendObject) {
createFromBackendDict(
topicRightsBackendObject: {
published: boolean;
canPublishTopicBool: boolean;
canEditTopicBool: boolean;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to change can_publish_topic and similarly can_edit_topic since they were failing the camelCase rule once they were converted to classes. Further canPublishTopic can not be the name since that name is already taken up by a member function.

}) {
return new TopicRights(
topicRightsBackendObject.published,
topicRightsBackendObject.can_publish_topic,
topicRightsBackendObject.can_edit_topic
topicRightsBackendObject.canPublishTopicBool,
topicRightsBackendObject.canEditTopicBool
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,19 @@
* @fileoverview Tests for TopicRightsObjectFactory.
*/

import { TopicRightsObjectFactory } from
import { TopicRightsObjectFactory, TopicRights } from
'domain/topic/TopicRightsObjectFactory.ts';

describe('Topic rights object factory', () => {
let topicRightsObjectFactory: TopicRightsObjectFactory;
var sampleTopicRights = null;
let sampleTopicRights: TopicRights = null;

beforeEach(() => {
topicRightsObjectFactory = new TopicRightsObjectFactory();
var initialTopicRightsBackendObject = {
published: false,
can_edit_topic: true,
can_publish_topic: true
canEditTopicBool: true,
canPublishTopicBool: true
};

sampleTopicRights = topicRightsObjectFactory.createFromBackendDict(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
* @fileoverview Unit tests for the controller of the 'State Editor'.
*/

// TODO(YashJipkate) Remove the unnnecessary imports once the corresponding
// non-spec file is upgraded.
// TODO(YashJipkate) Remove the following block of unnnecessary imports once
// exploration-editor-tab.directive.ts is upgraded to Angular 8.
import { AnswerClassificationResultObjectFactory } from
'domain/classifier/AnswerClassificationResultObjectFactory.ts';
import { ClassifierObjectFactory } from
Expand All @@ -29,6 +29,7 @@ import { ExplorationFeaturesService } from
import { RuleObjectFactory } from 'domain/exploration/RuleObjectFactory.ts';
import { WrittenTranslationObjectFactory } from
'domain/exploration/WrittenTranslationObjectFactory.ts';
// ^^^ This block is to be removed.

require('App.ts');
require('pages/exploration-editor-page/services/exploration-states.service.ts');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
* @fileoverview Unit tests for the training data service.
*/

// TODO(YashJipkate) Remove the unnnecessary imports once the corresponding
// non-spec file is upgraded.
// TODO(YashJipkate) Remove the following block of unnnecessary imports once
// training-data.service.ts is upgraded to Angular 8.
import { AnswerClassificationResultObjectFactory } from
'domain/classifier/AnswerClassificationResultObjectFactory.ts';
import { ClassifierObjectFactory } from
Expand All @@ -27,6 +27,7 @@ import { ExplorationDraftObjectFactory } from
import { RuleObjectFactory } from 'domain/exploration/RuleObjectFactory.ts';
import { WrittenTranslationObjectFactory } from
'domain/exploration/WrittenTranslationObjectFactory.ts';
// ^^^ This block is to be removed.

require('App.ts');
require('domain/exploration/OutcomeObjectFactory.ts');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,14 @@
* @fileoverview Unit tests for the exploration history tab.
*/

// TODO(YashJipkate) Remove the unnnecessary imports once the corresponding
// non-spec file is upgraded.
// TODO(YashJipkate) Remove the following block of unnnecessary imports once
// history-tab.directive.ts is upgraded to Angular 8.
import { ExplorationDraftObjectFactory } from
'domain/exploration/ExplorationDraftObjectFactory.ts';
import { RuleObjectFactory } from 'domain/exploration/RuleObjectFactory.ts';
import { WrittenTranslationObjectFactory } from
'domain/exploration/WrittenTranslationObjectFactory.ts';
// ^^^ This block is to be removed.

require('pages/exploration-editor-page/history-tab/history-tab.directive.ts');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,12 @@
* @fileoverview Unit tests for the Compare versions Service.
*/

// TODO(YashJipkate) Remove the unnnecessary imports once the corresponding
// non-spec file is upgraded.
// TODO(YashJipkate) Remove the following block of unnnecessary imports once
// compare-versions.service.ts is upgraded to Angular 8.
import { RuleObjectFactory } from 'domain/exploration/RuleObjectFactory.ts';
import { WrittenTranslationObjectFactory } from
'domain/exploration/WrittenTranslationObjectFactory.ts';
// ^^^ This block is to be removed.

require(
'pages/exploration-editor-page/history-tab/services/' +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,11 @@
* of the exploration editor page.
*/

// TODO(YashJipkate) Remove the unnnecessary imports once the corresponding
// non-spec file is upgraded.
// TODO(YashJipkate) Remove the following block of unnnecessary imports once
// exploration-rights.service.ts is upgraded to Angular 8.
import { ExplorationDraftObjectFactory } from
'domain/exploration/ExplorationDraftObjectFactory.ts';
// ^^^ This block is to be removed.

require('pages/exploration-editor-page/services/exploration-rights.service.ts');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
* @fileoverview Tests for ExplorationStatesService.
*/

// TODO(YashJipkate) Remove the unnnecessary imports once the corresponding
// non-spec file is upgraded.
// TODO(YashJipkate) Remove the following block of unnnecessary imports once
// exploration-states.service.ts is upgraded to Angular 8.
import { AnswerClassificationResultObjectFactory } from
'domain/classifier/AnswerClassificationResultObjectFactory.ts';
import { ClassifierObjectFactory } from
Expand All @@ -26,6 +26,7 @@ import { ExplorationDraftObjectFactory } from
'domain/exploration/ExplorationDraftObjectFactory.ts';
import { WrittenTranslationObjectFactory } from
'domain/exploration/WrittenTranslationObjectFactory.ts';
// ^^^ This block is to be removed.

require('components/state-editor/state-editor-properties-services/' +
'state-solicit-answer-details.service.ts');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
* jasmine.any(Object).
*/

// TODO(YashJipkate) Remove the unnnecessary imports once the corresponding
// non-spec file is upgraded.
// TODO(YashJipkate) Remove the following block of unnnecessary imports once
// learner-action-render.service.ts is upgraded to Angular 8.
import { ExplorationFeaturesService } from
'services/ExplorationFeaturesService.ts';
import { AnswerClassificationResultObjectFactory } from
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,12 @@
* @fileoverview Unit tests for statistics services.
*/

// TODO(YashJipkate) Remove the unnnecessary imports once the corresponding
// non-spec file is upgraded.
// TODO(YashJipkate) Remove the following block of unnnecessary imports once
// state-improvement-suggestion.service.ts is upgraded to Angular 8.
import { RuleObjectFactory } from 'domain/exploration/RuleObjectFactory.ts';
import { WrittenTranslationObjectFactory } from
'domain/exploration/WrittenTranslationObjectFactory.ts';
// ^^^ This block is to be removed.

require('domain/exploration/StatesObjectFactory.ts');
require(
Expand Down
Loading