Skip to content

Show PL Program Info Page even without Programs Offered#49586

Merged
TurnerRiley merged 7 commits into
stagingfrom
show-RP-info-without-programs
Jan 3, 2023
Merged

Show PL Program Info Page even without Programs Offered#49586
TurnerRiley merged 7 commits into
stagingfrom
show-RP-info-without-programs

Conversation

@TurnerRiley

@TurnerRiley TurnerRiley commented Dec 16, 2022

Copy link
Copy Markdown
Contributor

Revert fix

This is a recreation of this PR which was reverted due to .at() not being recognized as a function in the testing. I switched it to just use array square brackets to refer to that index's value to remove references to .at().

PR info

A bug on the Professional Learning Program Information page was discovered where no information was being shown for regional partners not offering a program at that time (which was throwing an error in console). It's a bug from this PR where it would try to load the workshop cards but not find any programs offered.

This PR just adds a check to make sure there are programs before trying to render cards for them. If there are no programs offered, it will just show the rest of the information without the workshop cards.

Bug behavior for no programs offered

Original not show PR info

With fix behavior for no programs offered

Show RP info

Links

Slack thread this was brought up: here

Testing story

Local testing.

PR Checklist:

  • Tests provide adequate coverage
  • Privacy and Security impacts have been assessed
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

@TurnerRiley TurnerRiley changed the title Show rp info without programs Show PL Program Info Page even without Programs Offered Dec 16, 2022
@TurnerRiley TurnerRiley requested a review from a team December 16, 2022 18:20
@megcrenshaw

Copy link
Copy Markdown

Hmm, interesting. Any idea why this wasn't coming up in our pipeline? Do you know if it's something that's been flagged? Would love for this to be on people's radar –– could you maybe ping infra about it?

It looks like it's fairly new syntax, but I'm concerned that it's not being caught in our Drone runs, or, better, locally.

@megcrenshaw

Copy link
Copy Markdown

Okay it looks like we're running 14.17.1 of node https://github.com/code-dot-org/code-dot-org/blob/staging/.nvmrc but we need at least 16.6.0 for the .at() function ... but I still am in the camp that it should be caught in our pipeline earlier?

@TurnerRiley

TurnerRiley commented Jan 3, 2023

Copy link
Copy Markdown
Contributor Author

Okay it looks like we're running 14.17.1 of node https://github.com/code-dot-org/code-dot-org/blob/staging/.nvmrc but we need at least 16.6.0 for the .at() function ... but I still am in the camp that it should be caught in our pipeline earlier?

Good point! It did get flagged in the apps tests in this thread. The .at() was only being used in the test file so it was just blocking the DTP rather than breaking anything on the site itself. I believe if the .at() function was being used in the non-test files the issue would have popped up sooner in development!

@TurnerRiley TurnerRiley merged commit f1baf9f into staging Jan 3, 2023
@TurnerRiley TurnerRiley deleted the show-RP-info-without-programs branch January 3, 2023 21:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants