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

fix: plugin working on clean installation #58

Merged
merged 1 commit into from
Nov 2, 2023

Conversation

julianramirez2
Copy link
Contributor

@julianramirez2 julianramirez2 commented Oct 30, 2023

Description

The issue of creating Enrollment Requests during a clean installation of WordPress and plugin installation was attributed to caching problems and checks when creating the table that holds vital request information in the WordPress database. After further investigation, the following errors were resolved:

  1. Misuse of $logs_table Variable: We identified an error where the variable $logs_table, obtained from the wp_cache_get() function, was incorrectly being used as the table name. This approach is flawed because the function does not return the table name. Using $logs_table for creating the table and conducting existence checks resulted in SQL errors.

  2. Incorrect Variable Usage: When the table was not found in the cache, SQL was employed to verify its existence in the database using 'SHOW TABLES LIKE.' Unfortunately, an error was made by using the variable $logs_table instead of the actual table name in the database.

  3. Table Creation Error: In the CREATE TABLE statement, we were using the variable $logs_table, which did not contain the correct table name to be created. To rectify this, we introduced the variable $logs_table_name, which combines the WordPress prefix and the table name.

  • Code Compliance: These corrections have been made to align the code with WordPress coding best practices.

This commit addresses these issues and improves the overall code reliability.

Testing instructions

To perform testing, you should bring the code from this branch to your local environment, run the 'composer install' command to fetch the libraries, and carry out a clean WordPress installation. Once you have a clean WordPress setup, use a .zip file containing the code we have in your local environment and install the plugin. Finally, verify its proper functionality.

Checklist for Merge

  • Tested in a remote environment
  • Updated documentation N/A
  • Rebased master/main N/A
  • Squashed commits N/A

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Oct 30, 2023
@openedx-webhooks
Copy link

openedx-webhooks commented Oct 30, 2023

Thanks for the pull request, @julianramirez2! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

@mphilbrick211
Copy link

Hi @julianramirez2! Your CLA check should be all set by tomorrow (Weds. Nov 1). Please let me know if you're still having any issues.

Copy link
Contributor

@MaferMazu MaferMazu left a comment

Choose a reason for hiding this comment

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

Note: the plugin was weirdly using the cache, and you couldn't see the logs if you installed the plugin for the first time.

Thanks for this fix, @julianramirez2; it works.

@MaferMazu MaferMazu force-pushed the plugin-activation-fix branch from 241b522 to 0ac56ee Compare November 2, 2023 20:41
@MaferMazu MaferMazu merged commit f530acd into openedx:main Nov 2, 2023
3 checks passed
@openedx-webhooks
Copy link

@julianramirez2 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
open-source-contribution PR author is not from Axim or 2U
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants