Fix part of #9749: Migrate I18nFooter Directive #12925

ashutoshc8101 · 2021-05-26T15:11:22Z

Overview

This PR fixes part of Migrate directives/controllers to Angular components #9749.
This PR does the following: Migrate I18nFooter Directive.

Note: This PR is independent of #12790.

Essential Checklist

The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes. (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
The linter/Karma presubmit checks have passed locally on your machine.
"Allow edits from maintainers" is checked. (See here for instructions on how to enable it.)
- This lets reviewers restart your CircleCI tests for you.
The PR is made from a branch that's not called "develop".

Proof that changes are correct

i18n-footer-2021-05-26_20.43.42.mp4

The Follow Us text in the footer belongs to an angular component whereas about, teach and contribute sections belong to an angular js directive.

So, this works in both.

PR Pointers

Make sure to follow the instructions for making a code change.
Oppiabot will notify you when you don't add a PR_CHANGELOG label. If you are unable to do so, please @-mention a code owner (who will be in the Reviewers list), or ask on Gitter.
For what code owners will expect, see the Code Owner's wiki page.
Make sure your PR follows conventions in the style guide, otherwise this will lead to review delays.
Never force push. If you do, your PR will be closed.
Oppiabot can assign anyone for review/help if you leave a comment like the following: "{{Question/comment}} @{{reviewer_username}} PTAL"
Some of the e2e tests are flaky, and can fail for reasons unrelated to your PR. We are working on fixing this, but in the meantime, if you need to restart the tests, please check the "If your build fails" wiki page.

oppiabot · 2021-05-26T15:11:23Z

Assigning @srijanreddy98 for the first-pass review of this pull request. Thanks!

oppiabot · 2021-05-26T15:11:26Z

Hi @ashutoshc8101, can you complete the following:

The proof that changes are correct has not been provided, please make sure to upload a image/video showing that the changes are correct. Or include a sentence saying "No proof of changes needed because" and the reason why proof of changes cannot be provided.
Thanks!

oppiabot · 2021-05-27T07:47:23Z

Hi @ashutoshc8101. Due to recent changes in the "develop" branch, this PR now has a merge conflict. Please follow this link if you need help resolving the conflict, so that the PR can be merged. Thanks!

srijanreddy98

LGTM -- thanks @ashutoshc8101!

oppiabot · 2021-05-28T03:38:43Z

Assigning @vojtechjelinek, @kevintab95, @DubeySandeep, @jameesjohn, @darksun27 for code owner reviews. Thanks!

ashutoshc8101 · 2021-05-30T11:59:42Z

@DubeySandeep Thank You for your review.
I have made the suggested changes and addressed a comment.
PTAL

oppiabot · 2021-05-30T12:02:46Z

Unassigning @ashutoshc8101 since a re-review was requested. @ashutoshc8101, please make sure you have addressed all review comments. Thanks!

vojtechjelinek

Thanks! Left a few comments.

vojtechjelinek · 2021-05-30T15:56:07Z

core/templates/base-components/i18n-language-selector.component.ts

+  readonly id: string,
+  readonly text: string,
+  readonly direction: string


Suggested change

readonly id: string,

readonly text: string,

readonly direction: string

readonly id: string;

readonly text: string;

readonly direction: string;

vojtechjelinek · 2021-05-30T15:57:06Z

core/templates/services/user-backend-api.service.ts

+  async updatePreferredSiteLanguageAsync(currentLanguageCode: string):
+  Promise<Object> {


Suggested change

async updatePreferredSiteLanguageAsync(currentLanguageCode: string):

Promise<Object> {

async updatePreferredSiteLanguageAsync(

currentLanguageCode: string

): Promise<Object> {

oppiabot · 2021-05-30T16:00:25Z

Hi @ashutoshc8101, it looks like some changes were requested on this pull request by @vojtechjelinek. PTAL. Thanks!

ashutoshc8101 · 2021-05-30T16:21:49Z

@vojtechjelinek Thank you for your review.
I have made the suggested changes.
PTAL

oppiabot · 2021-05-30T16:24:52Z

Unassigning @ashutoshc8101 since a re-review was requested. @ashutoshc8101, please make sure you have addressed all review comments. Thanks!

DubeySandeep

LGTM! (Left one nit comment, PTAL!)

DubeySandeep · 2021-05-30T17:46:38Z

core/templates/services/user-backend-api.service.ts

@@ -57,16 +57,17 @@ export interface UserContributionRightsDataBackendDict {
  providedIn: 'root'
 })
 export class UserBackendApiService {
-  constructor(private http: HttpClient) {}
+  constructor(private httpClient: HttpClient) {}


why this change? (I think we have the old pattern everywhere in the codebase)

Oh! I have been doing this in all of my PRs. Following the convention of the first letter lowercase for the object name.
Looking deeply into the codebase, looks like we are going with http in most places. Reverting this change.

oppiabot · 2021-05-30T17:52:10Z

Unassigning @DubeySandeep since they have already approved the PR.

oppiabot · 2021-05-31T15:08:31Z

Hi @ashutoshc8101. Due to recent changes in the "develop" branch, this PR now has a merge conflict. Please follow this link if you need help resolving the conflict, so that the PR can be merged. Thanks!

ashutoshc8101 · 2021-06-01T09:00:47Z

@vojtechjelinek PTAL

vojtechjelinek

LGTM!

ashutoshc8101 added 4 commits May 26, 2021 20:20

migrated

747689a

nit

3ff821e

nit

9fd8ec5

nits

d6ce522

ashutoshc8101 added the PR CHANGELOG: Angular Migration -- @srijanreddy98 label May 26, 2021

ashutoshc8101 requested review from darksun27, DubeySandeep, jameesjohn, kevintab95, srijanreddy98 and vojtechjelinek as code owners May 26, 2021 15:11

oppiabot bot assigned srijanreddy98 May 26, 2021

oppiabot bot assigned ashutoshc8101 May 26, 2021

ashutoshc8101 removed their assignment May 26, 2021

ashutoshc8101 mentioned this pull request May 26, 2021

Flake Report: Backend tests. #12928

Closed

nit

cc53923

oppiabot bot added the PR: don't merge - HAS MERGE CONFLICTS label May 27, 2021

oppiabot bot assigned ashutoshc8101 May 27, 2021

Merge remote-tracking branch 'upstream/develop' into i18nFooterFinal

4cc817f

oppiabot bot removed the PR: don't merge - HAS MERGE CONFLICTS label May 27, 2021

srijanreddy98 approved these changes May 28, 2021

View reviewed changes

srijanreddy98 removed their assignment May 28, 2021

oppiabot bot assigned darksun27, DubeySandeep, jameesjohn and kevintab95 May 28, 2021

ashutoshc8101 added 2 commits May 30, 2021 17:25

nit

38d4561

Merge remote-tracking branch 'upstream/develop' into i18nFooterFinal

edb4f4e

oppiabot bot removed the PR: don't merge - HAS MERGE CONFLICTS label May 30, 2021

oppiabot bot assigned DubeySandeep and unassigned ashutoshc8101 May 30, 2021

vojtechjelinek requested changes May 30, 2021

View reviewed changes

vojtechjelinek removed their assignment May 30, 2021

oppiabot bot assigned ashutoshc8101 May 30, 2021

done

49e9fd0

oppiabot bot assigned vojtechjelinek and unassigned ashutoshc8101 May 30, 2021

DubeySandeep approved these changes May 30, 2021

View reviewed changes

oppiabot bot unassigned DubeySandeep May 30, 2021

nit

cbd3bf5

oppiabot bot added the PR: don't merge - HAS MERGE CONFLICTS label May 31, 2021

oppiabot bot assigned ashutoshc8101 May 31, 2021

Merge remote-tracking branch 'upstream/develop' into i18nFooterFinal

82bb9e2

oppiabot bot removed the PR: don't merge - HAS MERGE CONFLICTS label Jun 1, 2021

vojtechjelinek approved these changes Jun 1, 2021

View reviewed changes

vojtechjelinek removed their assignment Jun 1, 2021

ashutoshc8101 enabled auto-merge (squash) June 1, 2021 10:28

ashutoshc8101 merged commit 3dc78ce into oppia:develop Jun 1, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix part of #9749: Migrate I18nFooter Directive #12925

Fix part of #9749: Migrate I18nFooter Directive #12925

ashutoshc8101 commented May 26, 2021 •

edited

Loading

oppiabot bot commented May 26, 2021

oppiabot bot commented May 26, 2021

oppiabot bot commented May 27, 2021

srijanreddy98 left a comment

oppiabot bot commented May 28, 2021

ashutoshc8101 commented May 30, 2021

oppiabot bot commented May 30, 2021

vojtechjelinek left a comment

vojtechjelinek May 30, 2021

ashutoshc8101 May 30, 2021

vojtechjelinek May 30, 2021

ashutoshc8101 May 30, 2021

oppiabot bot commented May 30, 2021

ashutoshc8101 commented May 30, 2021

oppiabot bot commented May 30, 2021

DubeySandeep left a comment

DubeySandeep May 30, 2021

ashutoshc8101 May 30, 2021

ashutoshc8101 May 30, 2021

oppiabot bot commented May 30, 2021

oppiabot bot commented May 31, 2021

ashutoshc8101 commented Jun 1, 2021

vojtechjelinek left a comment

		async updatePreferredSiteLanguageAsync(currentLanguageCode: string):
		Promise<Object> {

Fix part of #9749: Migrate I18nFooter Directive #12925

Fix part of #9749: Migrate I18nFooter Directive #12925

Conversation

ashutoshc8101 commented May 26, 2021 • edited Loading

Overview

Essential Checklist

Proof that changes are correct

PR Pointers

oppiabot bot commented May 26, 2021

oppiabot bot commented May 26, 2021

oppiabot bot commented May 27, 2021

srijanreddy98 left a comment

Choose a reason for hiding this comment

oppiabot bot commented May 28, 2021

ashutoshc8101 commented May 30, 2021

oppiabot bot commented May 30, 2021

vojtechjelinek left a comment

Choose a reason for hiding this comment

vojtechjelinek May 30, 2021

Choose a reason for hiding this comment

ashutoshc8101 May 30, 2021

Choose a reason for hiding this comment

vojtechjelinek May 30, 2021

Choose a reason for hiding this comment

ashutoshc8101 May 30, 2021

Choose a reason for hiding this comment

oppiabot bot commented May 30, 2021

ashutoshc8101 commented May 30, 2021

oppiabot bot commented May 30, 2021

DubeySandeep left a comment

Choose a reason for hiding this comment

DubeySandeep May 30, 2021

Choose a reason for hiding this comment

ashutoshc8101 May 30, 2021

Choose a reason for hiding this comment

ashutoshc8101 May 30, 2021

Choose a reason for hiding this comment

oppiabot bot commented May 30, 2021

oppiabot bot commented May 31, 2021

ashutoshc8101 commented Jun 1, 2021

vojtechjelinek left a comment

Choose a reason for hiding this comment

ashutoshc8101 commented May 26, 2021 •

edited

Loading