enable and disable SUPPORT_URL with environment variable #276

isankadn · 2022-11-05T17:22:57Z

Description:

On the MFE learning top menu, the "Help" link is currently displayed by default.

With this change, the "Help" menu will only be visible if the "SUPPORT URL" variable is not empty.

openedx-webhooks · 2022-11-05T17:23:05Z

Thanks for the pull request, @isankadn! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

supporting documentation
Open edX discussion forum threads
timeline information ("this must be merged by XX date", and why that is)
partner information ("this is a course on edx.org")
any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

arbrandes

Thanks @isankadn! I haven't actually tested this yet, but in the meantime, can you please:

Create a PR to add SUPPORT_URL to the .env* files in frontend-template-application
Add an explanation of what this does to this repo's README?

isankadn · 2022-11-09T01:29:24Z

@arbrandes Thank you very much!
Both items are now completed!

Here is the pull request to frontend-template-application openedx/frontend-template-application#499

DeanJayMathew · 2022-11-29T15:30:36Z

Thanks for helping @arbrandes !

arbrandes · 2022-11-30T12:31:01Z

@isankadn, thanks!

However, it looks like we're failing some tests. Can you look into it?

Also, please refer to the automated message above: we need you to sign the CLA before accepting your code. Mind doing that? Thanks!

isankadn · 2022-12-01T06:58:38Z

@arbrandes Thank you for your comment, sure I can sign. and the same time, I'm representing ABC for this, I think @DeanJayMathew has already taken care of that.

and I will look into this error as well. thx.

Also, please refer to the automated message above: we need you to sign the CLA before accepting your code. Mind doing that? Thanks!

isankadn · 2022-12-01T07:41:54Z

@arbrandes fixed the linting issue. and pushed.

codecov · 2022-12-01T17:22:06Z

Codecov Report

Base: 61.87% // Head: 62.12% // Increases project coverage by +0.25% 🎉

Coverage data is based on head (7e759b7) compared to base (536d674).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #276      +/-   ##
==========================================
+ Coverage   61.87%   62.12%   +0.25%     
==========================================
  Files          15       15              
  Lines         299      301       +2     
  Branches       70       71       +1     
==========================================
+ Hits          185      187       +2     
  Misses        112      112              
  Partials        2        2

Impacted Files	Coverage Δ
src/learning-header/AuthenticatedUserDropdown.jsx	`87.50% <100.00%> (+4.16%)`	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

mphilbrick211 · 2023-01-06T18:42:13Z

Hi @isankadn! Just following up on this PR. In order to push it through, you'll need to complete a CLA form. Please let me know if you have any questions.

mphilbrick211 · 2023-01-11T01:37:59Z

Hi @isankadn! Tests have been enabled for this pull request, though you'll still need to fill out a CLA form. Please let us know if you plan to pursue this pull request. Thanks!

DeanJayMathew · 2023-01-11T02:02:48Z

Hi @michelle Philbrick ***@***.***> Thank you for following up on this. @isanka Wijerathne ***@***.***> ( https://github.com/isankadn, ***@***.***) should be covered under the organisational contributors agreement I signed on behalf of ABC ONLINE COURSES LIMITED. Dean Jay Mathew Director, ABC Online Courses Limited Trusted Open edX® Service Providers since 2015 Schedule a meeting with me: https://calendly.com/abconlinecourses View Our Website <http://www.abconlinecourses.com> Tel.: +441225293341 Registered Address: Chestnut House ***@***.***,-2.1459131,15z/data=!3m1!4b1!4m8!1m2!2m1!1schestnut+house+53d+beanacre+sn127py!3m4!1s0x48717c9105a9d3d7:0x15804f821ad57207!8m2!3d51.3980641!4d-2.1374675> (Wiltshire, England, UK) *Legal Disclaimer: **This email and any files transmitted with it are confidential and intended solely for the use of the individual or entity to whom they are addressed and intended. If you have received this email in error please notify the sender or system manager immediately. This message contains confidential information and is intended only for the individual named. If you are not the named addressee you should not disseminate, distribute or copy this e-mail. Please notify the sender immediately by e-mail if you have received this e-mail by mistake and delete this e-mail from your system. If you are not the intended recipient you are notified that disclosing, copying, distributing or taking any action in reliance on the contents of this information is strictly prohibited.*

…

On Wed, Jan 11, 2023 at 8:38 AM Michelle Philbrick ***@***.***> wrote: Hi @isankadn <https://github.com/isankadn>! Tests have been enabled for this pull request, though you'll still need to fill out a CLA form <https://openedx.atlassian.net/wiki/spaces/COMM/pages/941457737/How+to+start+contributing+to+the+Open+edX+code+base>. Please let us know if you plan to pursue this pull request. Thanks! — Reply to this email directly, view it on GitHub <#276 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACMGDW6QAMVOXC24ZKGVGZ3WRYFIFANCNFSM6AAAAAARYAOAWU> . You are receiving this because you were mentioned.Message ID: ***@***.***>

mphilbrick211 · 2023-01-11T22:04:01Z

Thanks @DeanJayMathew! I'm not seeing @isankadn listed on our end. Let me follow-up internally to make sure I'm not missing anything.

e0d · 2023-01-20T22:22:02Z

@mphilbrick211 I see him in the CLA file now.

@isankadn I also notice there are some commit lint issues, we use conventional commits as defined here: https://open-edx-proposals.readthedocs.io/en/latest/best-practices/oep-0051-bp-conventional-commits.html#type

mphilbrick211 · 2023-01-23T20:36:02Z

Hi @isankadn! Just flagging Ed's comment and Adolfo's requested changes. Also, your CLA should be all set now, so once tests are re-run the check should turn green.

arbrandes · 2023-01-24T13:55:57Z

src/learning-header/AuthenticatedUserDropdown.jsx

+  const SupportUrl = (
+    <a className="text-gray-700 mr-3" href={`${getConfig().SUPPORT_URL}`}>{intl.formatMessage(messages.help)}</a>
+  );


I don't think we need to specify a separate variable for this. It's fine if we just do it below, via:

{getConfig().SUPPORT_URL && ( <a>...</a> )}

arbrandes · 2023-01-24T13:57:17Z

@isankadn after you address my request, can you please squash your commits into a single one with a conventional commit title? In this case it would be a feat:.

mphilbrick211 · 2023-02-07T22:16:59Z

Hi @isankadn! Just checking in on this to see if you've been able to address Adolfo's feedback. Please let us know when this is ready to review. Thank you!

mphilbrick211 · 2023-02-24T03:12:51Z

Hi @isankadn! Just checking in on this to see if you've been able to address Adolfo's feedback. Please let us know when this is ready to review. Thank you!

Hi @isankadn! Just checking in to see if you are able to re-run the failing tests and confirm if the requested changes have been made. Thanks!

mphilbrick211 · 2023-03-24T21:21:52Z

Hi @isankadn! Just checking in on this to see if you've been able to address Adolfo's feedback. Please let us know when this is ready to review. Thank you!

Hi @isankadn! Just checking in to see if you are able to re-run the failing tests and confirm if the requested changes have been made. Thanks!

Hi @isankadn friendly ping on this. Are you planning to pursue this PR?

mphilbrick211 · 2023-04-04T01:52:11Z

Hi @isankadn - just checking in to see if you plan to pursue this pull request - please let us know. Thanks!

mphilbrick211 · 2023-04-25T00:29:53Z

Hi @isankadn - I'm going to close this pull request for now. We can reopen if you'd like to pursue in the future. Thanks!

openedx-webhooks · 2023-04-25T00:29:59Z

@isankadn Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future.

isankadn added 3 commits November 5, 2022 20:37

SUPPORT_URL switch

645d87c

enable and disabled SUPPORT_URL with env variable

918a1ed

SUPPORT_URLrelated formatting

4643dd8

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Nov 5, 2022

arbrandes requested changes Nov 7, 2022

View reviewed changes

arbrandes mentioned this pull request Nov 7, 2022

Reviews for sprint 2022-11-11 openedx/axim-engineering#518

Closed

30 tasks

SUPPORT_URL related; Updated .env.development file and README.rst.

c0ba815

isankadn mentioned this pull request Nov 9, 2022

Isanka/dev openedx/frontend-template-application#499

Closed

arbrandes mentioned this pull request Nov 14, 2022

Reviews for sprint 2022-11-25 openedx/axim-engineering#539

Closed

36 tasks

arbrandes mentioned this pull request Nov 29, 2022

Reviews for sprint starting on 2022-11-28 openedx/axim-engineering#553

Closed

28 tasks

isankadn added 2 commits December 1, 2022 16:25

fixing lining issues

717e164

Merge branch 'openedx:master' into isanka/help_link_switch

7e759b7

arbrandes mentioned this pull request Dec 21, 2022

Reviews for sprint 2023-01-09 - 2023-01-22 openedx/axim-engineering#595

Closed

22 tasks

arbrandes mentioned this pull request Jan 18, 2023

Adolfo's reviews (2023-01-23) openedx/axim-engineering#620

Closed

44 tasks

e0d added the waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. label Jan 20, 2023

arbrandes requested changes Jan 24, 2023

View reviewed changes

arbrandes mentioned this pull request Feb 6, 2023

Adolfo's reviews (2023-02-06) openedx/axim-engineering#655

Closed

22 tasks

This was referenced Mar 1, 2023

Adolfo's reviews (2023-02-20) openedx/axim-engineering#688

Closed

Adolfo's reviews openedx/axim-engineering#696

Closed

mphilbrick211 added the inactive PR author has been unresponsive for several months label Apr 4, 2023

mphilbrick211 added closed inactivity PR was closed because the author abandoned it and removed waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. inactive PR author has been unresponsive for several months labels Apr 25, 2023

mphilbrick211 closed this Apr 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

enable and disable SUPPORT_URL with environment variable #276

enable and disable SUPPORT_URL with environment variable #276

isankadn commented Nov 5, 2022 •

edited

Loading

openedx-webhooks commented Nov 5, 2022 •

edited

Loading

arbrandes left a comment •

edited

Loading

isankadn commented Nov 9, 2022 •

edited

Loading

DeanJayMathew commented Nov 29, 2022

arbrandes commented Nov 30, 2022

isankadn commented Dec 1, 2022

isankadn commented Dec 1, 2022

codecov bot commented Dec 1, 2022 •

edited

Loading

mphilbrick211 commented Jan 6, 2023

mphilbrick211 commented Jan 11, 2023

DeanJayMathew commented Jan 11, 2023 via email

mphilbrick211 commented Jan 11, 2023

e0d commented Jan 20, 2023

mphilbrick211 commented Jan 23, 2023

arbrandes Jan 24, 2023

arbrandes commented Jan 24, 2023

mphilbrick211 commented Feb 7, 2023

mphilbrick211 commented Feb 24, 2023

mphilbrick211 commented Mar 24, 2023

mphilbrick211 commented Apr 4, 2023

mphilbrick211 commented Apr 25, 2023

openedx-webhooks commented Apr 25, 2023

enable and disable SUPPORT_URL with environment variable #276

enable and disable SUPPORT_URL with environment variable #276

Conversation

isankadn commented Nov 5, 2022 • edited Loading

openedx-webhooks commented Nov 5, 2022 • edited Loading

arbrandes left a comment • edited Loading

Choose a reason for hiding this comment

isankadn commented Nov 9, 2022 • edited Loading

DeanJayMathew commented Nov 29, 2022

arbrandes commented Nov 30, 2022

isankadn commented Dec 1, 2022

isankadn commented Dec 1, 2022

codecov bot commented Dec 1, 2022 • edited Loading

Codecov Report

mphilbrick211 commented Jan 6, 2023

mphilbrick211 commented Jan 11, 2023

DeanJayMathew commented Jan 11, 2023 via email

mphilbrick211 commented Jan 11, 2023

e0d commented Jan 20, 2023

mphilbrick211 commented Jan 23, 2023

arbrandes Jan 24, 2023

Choose a reason for hiding this comment

arbrandes commented Jan 24, 2023

mphilbrick211 commented Feb 7, 2023

mphilbrick211 commented Feb 24, 2023

mphilbrick211 commented Mar 24, 2023

mphilbrick211 commented Apr 4, 2023

mphilbrick211 commented Apr 25, 2023

openedx-webhooks commented Apr 25, 2023

isankadn commented Nov 5, 2022 •

edited

Loading

openedx-webhooks commented Nov 5, 2022 •

edited

Loading

arbrandes left a comment •

edited

Loading

isankadn commented Nov 9, 2022 •

edited

Loading

codecov bot commented Dec 1, 2022 •

edited

Loading