feat: Certificate sharing to linkedin optionally consider course leve…

[haftamuk]

Description

When learners complete a course and generate certificate they have an option to share it to social media including LinkedIn, Twitter, Facebook. Previously, when sharing to LinkedIn, sharing form will be auto populated following THIS documentation. But regardless of course specific registered organizations( in a given open edx instance courses could be associated with different organizations, may be related to ownership!), platform name or platform LinkedIn ID are considered. This issue provides additional option to share course specific organization names to be shared.

Operators will have to enable SOCIAL_SHARING_SETTINGS and set CERTIFICATE_LINKEDIN_DEFAULTS_TO_COURSE_ORGANIZATION_NAME to True inorder for learners to be able to share their certificates in LinkedIn with course level organization.

More explanation can be obtained HERE

Supporting information

#36975

[openedx-webhooks]

Thanks for the pull request, @haftamuk!

This repository is currently maintained by @openedx/wg-maintenance-edx-platform.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

• If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
  • This process (including the steps you'll need to take) is documented here.
• If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

• Dependencies

  This PR must be merged before / after / at the same time as ...

• Blockers

  This PR is waiting for OEP-1234 to be accepted.

• Timeline information

  This PR must be merged by XX date because ...

• Partner information

  This is for a course on edx.org.

• Supporting documentation
• Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details

---

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

• Overview of Review Process for Community Contributions
• Pull Request Status Guide
• Making changes to your pull request

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

• The size and impact of the changes that it introduces
• The need for product review
• Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

[deborahgu]

I put this on my list to review, but in the meantime, could you look into the failing checks? If you need me to rerun anything in the PR just let me know here. Thanks!

[haftamuk]

@deborahgu Kindly rerun the unit tests.

[deborahgu]

@haftamuk done.

[Cup0fCoffee]

@haftamuk Please fix the formatting of the PR title, description and commit message. Also, please make sure that all checks are passing. Currently there are several quality checks failing, such as code linting and some tests related to code that your changes are touching.

[deborahgu]

I am annoyed (not at you!) by having all this logic in the model, instead of in a utility file (and honestly, this should be in the certificates app and not in the student app), but that's just me whining, I'm not asking you to make that change in what's obviously a legacy problem.

[deborahgu]

this method (_organization_information()) is used in exactly one place right now, namely, right here. Rather than having a chunk of logic here that says

if we have this setting enabled
     get the course org name, 
else, 
     go to this private method which says
     if we have a linkedIn org ID
          use it
     else
           use org name

why not just have one private method that figures out what to use here?

eg.

    def _get_organization_for_cert_share(self, course):
        share_settings = configuration_helpers.get_value('SOCIAL_SHARING_SETTINGS', settings.SOCIAL_SHARING_SETTINGS)
        prefer_course_organization_name = share_settings.get('CERTIFICATE_LINKEDIN_DEFAULTS_TO_COURSE_ORGANIZATION_NAME', False)    
        if prefer_course_organization_name:
           return {'organizationName', course.display_organization}
            })
        else:
            org_id = configuration_helpers.get_value('LINKEDIN_COMPANY_ID', self.company_identifier)
            # Prefer organization ID per documentation at https://addtoprofile.linkedin.com/
            if org_id:
                return {'organizationId': org_id}
            return {'organizationName': configuration_helpers.get_value('platform_name', settings.PLATFORM_NAME)}

[haftamuk]

This is more cleaner way, I will adjust accordingly.

[deborahgu]

Suggested change

	prefere_course_organization_name = share_settings.get('CERTIFICATE_LINKEDIN_DEFAULTS_TO_COURSE_ORGANIZATION_NAME', False)
	if prefere_course_organization_name:
	prefer_course_organization_name = share_settings.get('CERTIFICATE_LINKEDIN_DEFAULTS_TO_COURSE_ORGANIZATION_NAME', False)
	if prefer_course_organization_name:

[deborahgu]

we shouldn't make the same configuration getter call multiple times in the same process, and the line

share_settings = configuration_helpers.get_value('SOCIAL_SHARING_SETTINGS', settings.SOCIAL_SHARING_SETTINGS)

is already called 3 times in this class. Could you make a class init that sets this on self so we only have to call it once?

[haftamuk]

Very well observed! I will make the adjustment.

[deborahgu]

I wonder if we should make it clear somewhere that this means that there won't be a link to the organization ID, which will make LinkedIn not hyperlink from a certificate to an institution's own LinkedIn account. Arguably we could add LINKEDIN_ORGANIZATION_ID as an optional field on the organization table, although that does seem like it might lead to option fatigue. Maybe nobody really cares about this concern?

[haftamuk]

I actually had this considered an option in my comment HERE

This would be nice to have but I thought may be after a clean resolution of the current issue we can the proceed for more. Unless you think we should address that now?

[haftamuk]

@deborahgu I have found it difficult to resolve the issues in this PR and (I think mostly accidentally!) have closed this issue. I have continued the same work in the same branch in this PR. If you can easily adjust this would be appreciated other wise I hope it won't be a problem to work in the new open PR.