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

Umar/5124 create external resource with add links #2279

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

umar8hassan
Copy link
Contributor

@umar8hassan umar8hassan commented Aug 22, 2024

What are the relevant tickets?

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

Description (What does it do?)

This change is aimed to add External Resources on Link Add/Update. The syntax used for Link will be replaced with syntax for Resources in Markdown i.e. {{ % resource_link "uuid" "title" % }}

How can this be tested?

  1. Switch to umar/5124-create-external-resource-with-add-links branch in ocw-studio
  2. Open text editor
  3. Add a link
Screenshot 2024-08-23 at 11 01 13 AM 4. The syntax of link should be replaced with resource syntax in editor Screenshot 2024-08-23 at 10 58 04 AM 5. The resource should not be editable Screenshot 2024-08-23 at 11 01 20 AM
  1. Type a URL and upon space/enter, it should convert to a Resource Link
  • If the same URL as above, verify that the existing resource ID is being used.
  • If a different URL a new resource id will be used
  1. verify the use case without selecting any text and adding a link with the AddLink button

Additional Testing

Enable/disable OCW_STUDIO_CUSTOM_LINKUI_ENABLE feature flag on PostHog to enable/disable the feature for users.
Setup PostHog with instruction in the Readme.

@umar8hassan umar8hassan self-assigned this Aug 22, 2024
@umar8hassan umar8hassan marked this pull request as ready for review August 23, 2024 06:15
@ibrahimjaved12 ibrahimjaved12 self-requested a review August 23, 2024 13:25
Copy link
Contributor

@ibrahimjaved12 ibrahimjaved12 left a comment

Choose a reason for hiding this comment

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

Looks good to me

@pdpinch
Copy link
Member

pdpinch commented Aug 26, 2024

Is this behind a (posthog) feature flag? I want to be able to control the roll-out of this feature, limiting it to certain users until it's been thoroughly tested.

@pdpinch
Copy link
Member

pdpinch commented Sep 10, 2024

Is this blocked by #2291 ?

@umar8hassan
Copy link
Contributor Author

Is this blocked by #2291 ?

Yes. It was. But I can rebase on the branch and work on it now, since PostHog has been integrated with core functionality. Additional changes for PostHog integration will be done later in separate tickets.

@umar8hassan umar8hassan force-pushed the umar/5124-create-external-resource-with-add-links branch from 0500832 to 2c53647 Compare September 13, 2024 12:33
Copy link
Contributor

@ibrahimjaved12 ibrahimjaved12 left a comment

Choose a reason for hiding this comment

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

👍🏻

@umar8hassan umar8hassan self-assigned this Sep 22, 2024
@umar8hassan
Copy link
Contributor Author

Holding this since adding a link without selection ends up with both of Link and ResourceLink syntax being inserted. Need to resolve this before merging.

@pdpinch
Copy link
Member

pdpinch commented Sep 25, 2024

When fixing #2279 (comment) be sure to add a test

Otherwise this should get merged Thursday 9/26

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants