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

[Product Pull Request] Make course description editable in certs #227

Closed
4 of 7 tasks
jmakowski1123 opened this issue Feb 13, 2023 · 15 comments
Closed
4 of 7 tasks
Labels
product review complete PR has gone through product review

Comments

@jmakowski1123
Copy link

jmakowski1123 commented Feb 13, 2023

For Contributing Author:

This is the Primary Product Ticket for the following community contribution: Make course description editable in certificates

Checklist prior to undergoing Product Review:

The following information is required in order for Product Managers to be able to review your pull request:

  • Explanation of the problem being solved
  • Description of how users will be impacted, and which users will be impacted
  • Screenshots or video showing the functionality or fix, before and after
  • Reproduction steps and/or testing steps

Only if necessary:

Related PRs

For Product Manager doing the review:

What criteria should be analyzed from Product to approve a PR?

  • The problem being solved by the feature or fix is clear.
  • There is clarity on how the change or fix will impact the end user.
  • It is clear that the change will not negatively impact users or other areas of the platform.
  • The change is implemented comprehensively.
  • Any changes to UI use the current, standard Paragon Design System: https://paragon-openedx.netlify.app/
@github-actions
Copy link

Thanks for your submission, @openedx/open-edx-project-managers will review shortly.

@jmakowski1123
Copy link
Author

Product Info from the PR (29850)

Description

This PR makes the default description text that appears under the course title editable/configurable for certificates.

Supporting information

JIRA tickets: BB-5819

Screenshots

Creating new certificates:

certs-editable-field-during-creation

Testing instructions

  1. Check out this branch and open Studio.
  2. Enable certificates on the a course.
  3. On clicking "Set up your certificate", you should see the additional field "Course Description".
  4. Add in some text for course description and preview certificate.
  5. Verify that the custom course description is shown on the certificate.

@jmakowski1123
Copy link
Author

Product Review should be assigned to Claire Zhang.

@ProductRyan
Copy link

@czhang0912 do you have any questions about this idea? Any concerns about its effect on the user experience or if it conflicts with anything that you have planned?

@mphilbrick211
Copy link

Hi @czhang0912! Just flagging Kelly's comment / feedback for product here in the original ticket.

@mphilbrick211
Copy link

Hi @czhang0912! Just flagging Kelly's comment / feedback for product here in the original ticket.

Hi @czhang0912 - friendly ping on this!

@czhang0912
Copy link

@mphilbrick211 @ProductRyan As Kelly mentioned, we are current in the progress building the roadmap to extract the credentials from the LMS. We would like to focus on prioritizing the existing fields and not introducing another layer of complexities. Please let me know the use cases of this feature and if is critically needed for the future support in the long run.

@e0d
Copy link

e0d commented Jul 19, 2023

@pkulkark and @gabor-boros you both were involved in the PR. Could someone from the OpenCraft team provide a brief statement about the customer need that this work fulfills? What it does is pretty obvious, but we'd like to know

  • whether you are hearing about this from numerous customers?
  • was this a blocker for them or a nice-to-have?
  • are you carrying this as a patch for systems you are supporting?

@pkulkark
Copy link

@e0d Sure I can answer here. This came as a requirement for one of our clients. They are no longer with us but at the time of submitting the PR it was a blocker for them. They had multiple courses and had certificates configured for each of them. While all other fields in the certificate was configurable, the course description was hard-coded. We added this to our fork of edx-platform which we use for our clients.

@mphilbrick211
Copy link

@jmakowski1123 @e0d

Hi all - linking Kelly's most recent comment from the original PR here in case people didn't see.

@jmakowski1123
Copy link
Author

@pkulkark I'm copying over the feedback from one of the eng reviewers on the 2U side:

_"This is still very far from viable from my perspective. I would still like to see a clear set of requirements from Product and a sense of the level of need, but from there we would also need to work through several other questions about the particulars of how this feature should work, such as:

What does the field need to support? HTML? Django templating commands? Does it have a character limit?
How would this behave in existing templates? The OSPR updates the base templates but any existing templates in live instances will need to be manually updated to support this field.
Given that it requires updating all of your current templates to support this field, operators should have the ability to disable this feature if they do not plan to support it.
I think there is probably more that would come up if we thought through the requirements for this and really laid out all of the implications. But generally speaking, I don't feel this is production-ready as a feature."_

I don't think these are insurmountable questions, but given that this request came from a single client who is no longer with you, do you have access to the market input you'd need to define to this level of detail? Likewise, that change your relative prioritization of this enhancement?

@pkulkark
Copy link

pkulkark commented Sep 4, 2023

@jmakowski1123 No I do not have access to the market input needed. As I mentioned it was a request from just that one client. That being said, we did add this to our fork which is used by other clients so I'm not sure if others might be using it. My only additional comment is that the changes made here were made exactly like what's existing for Course Title. The same questions would be applicable to Course Title as well. Also this was meant to be a temporary solution until the course certificates were moved to credentials. So if there are reservations about it, I'm fine with closing the PR and reverting the changes on our fork. In case any of the other clients request it, we can reopen it.

@mphilbrick211
Copy link

Friendly ping on this @jmakowski1123!

@jmakowski1123
Copy link
Author

Given that this was initially implemented as a temporary solution until the Credentials work began, and it was initially built for the sake of one client among many, it's not a high priority enhancement to pursue now. @pkulkark If you find this creates a problem for multiple clients who may be using it without our knowledge, let me know and we can reconsider. Otherwise, closing this without pursuing for now.

@sarina
Copy link
Contributor

sarina commented Jun 4, 2024

Closing as this is marked "Abandoned" on the PR board. It may always be re-opened if needed.

@sarina sarina closed this as completed Jun 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product review complete PR has gone through product review
Projects
Archived in project
Status: Abandoned
Status: Ready for review
Development

No branches or pull requests

7 participants