Skip to content

v2 learning resource drawer formats and location #1826

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

Merged
merged 4 commits into from
Nov 15, 2024

Conversation

gumaerc
Copy link
Contributor

@gumaerc gumaerc commented Nov 14, 2024

What are the relevant tickets?

Closes https://github.com/mitodl/hq/issues/6019

Description (What does it do?)

This PR displays the format and location properties of a course or program in the info section of the new drawer. If any of items in location contain in_person, then the location data is displayed. If differing runs are detected, neither of these are displayed. If location is unavailable for in_person courses then it is simply not shown.

Screenshots (if appropriate):

image
image
image
image

How can this be tested?

  • If you have Posthog set up locally, enable the lr_drawer_v2 flag
  • If you don't have Posthog set up locally, you can force drawerV2 to be true in LearningResourceDrawer.tsx
  • Spin up this branch of mit-learn
  • Ensure you have the following courses in your database:
    • Leading the AI-Driven Organization
    • Communication and Persuasion in the Digital Age
  • Load the search page at http://localhost:8062/search
  • Search for "Leading the AI-Driven Organization"
  • Bring up the drawer and verify that "Format: In person" is shown with "Location: Cambridge, MA" below it
  • Close the drawer and search for Communication and Persuasion in the Digital Age
  • Bring up the drawer and verify that the differing runs table is shown, and the format and location info items are not shown

@gumaerc gumaerc added the Needs Review An open Pull Request that is ready for review label Nov 14, 2024
@gumaerc gumaerc force-pushed the cg/lr-drawer-v2-format-location branch from 6d68eb2 to 9a3d719 Compare November 15, 2024 17:04
@abeglova abeglova self-assigned this Nov 15, 2024
selector: (resource: LearningResource) => {
if (
shouldShowFormat(resource) &&
resource.delivery?.filter((d) => d.code === DeliveryEnum.InPerson)
Copy link
Contributor

@abeglova abeglova Nov 15, 2024

Choose a reason for hiding this comment

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

Should we show location for DeliveryEnum.Hybrid courses as well? Although i think all the mitpe hybrid courses have blank location currently. Also, should blank location for hybrid/in person courses default to cambridge?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sovsey What do you think?

Copy link

Choose a reason for hiding this comment

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

I think we should show Location for hybrid courses (for now at least). If the location is blank, the location field should not display. (I think the latter should be the behavior anyhow since @gumaerc you made an update that filters out info items entirely if they return empty or null content)

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 added in hybrid courses and merged this. Just to confirm @sovsey, this is set up to return null if location is missing, so yes that should work.

Copy link
Contributor

@abeglova abeglova left a comment

Choose a reason for hiding this comment

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

This looks good but I have a question about hybrid courses

@gumaerc gumaerc merged commit 7f2df1f into main Nov 15, 2024
11 checks passed
@odlbot odlbot mentioned this pull request Nov 18, 2024
15 tasks
jonkafton added a commit that referenced this pull request Nov 20, 2024
* Release 0.24.3

* Release date for 0.24.3

* Server rendered search page results

* v2 drawer certification updates (#1823)

* update certification display in v2 drawer to match latest designs

* don't show price info item if runs have differing data

* MicroMasters not Micromasters

* if there is no price for the certificate but it's indicated that one is included, display that

* if resource is free, includes a certification but has no prices, still display the pill in the info item

* generate migration for MicroMasters spelling change

* fix certificate pill padding on mobile

* Unit channel page and search prefetch

* Featured list and testimonials only for unit channels

* v2 learning resource drawer formats and location (#1826)

* add format info item

* display location if format is in_person

* add tests

* also show location for hybrid courses

* LocalDate and NoSSR components to render localized dates only on client

* Revert "LocalDate and NoSSR components to render localized dates only on client"

This reverts commit b4ccd6d.

* LocalDate and NoSSR components to render localized dates only on client (#1831)

* LocalDate and NoSSR components to render localized dates only on client

* Remove unnecessary React.Fragment

* separate starts and as taught in, show anytime availability (#1828)

* refactor starts / as taught in functionality to show on separate lines, show "anytime" in starts if availability is anytime

* fix rebase mishap

* Map address search params

* Search params test

* Update dependency pytest-cov to v6 (#1818)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Update dependency safety to v3 (#1819)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* URL search param validation anf transforms to align with course-search-utils

* Update dependency django-anymail to v12 (#1815)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* vector search endpoint (#1827)

* adding initial vector search view

* adding working vector results endpoint

* regenerate openapi spec

* fixing format of returned results

* adding test

* patching qdrant client

* moving to v0 api

* switch to custom serializer for vector search

* fix v0 url

* using minimal serializer

* returning minimal response for vector results

* regenerate spec

* adding some other useful bits to response

* fixing response for empty query and adjusting test

* regenerate spec

* uninheriting from searchrequest serializer

* updating oai spec

* updating oai spec

* Update dependency @mui/lab to v6.0.0-beta.15 (#1830)

* Update dependency @mui/lab to v6.0.0-beta.15

* update lockfile

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: shankar ambady <[email protected]>

* Update to use validators from course-search-utils

---------

Co-authored-by: Doof <[email protected]>
Co-authored-by: Carey P Gumaer <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Shankar Ambady <[email protected]>
Co-authored-by: shankar ambady <[email protected]>
mbertrand pushed a commit that referenced this pull request Nov 22, 2024
* add format info item

* display location if format is in_person

* add tests

* also show location for hybrid courses
@rhysyngsun rhysyngsun deleted the cg/lr-drawer-v2-format-location branch February 7, 2025 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review An open Pull Request that is ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants