Project

General

Profile

Support #13641

follow up to migration request

Added by Marilyn Weber about 1 year ago. Updated 11 months ago.

Status:
Resolved
Priority name:
Normal
Assignee:
Category name:
-
Target version:
Start date:
2020-12-01
Due date:
% Done:

0%

Estimated time:

Description

I'm opening this ticket just to connect Alan and Boone. LMK if I can help!

local.sql (693 KB) local.sql Alan Li, 2020-12-02 01:29 PM
cunygraduatelmis.zip (67.4 MB) cunygraduatelmis.zip Alan Li, 2020-12-04 02:08 AM
cuny-graduate-lmis.zip (28.1 MB) cuny-graduate-lmis.zip Alan Li, 2020-12-21 03:26 AM
cuny-graduate-lmis.zip (28.1 MB) cuny-graduate-lmis.zip Alan Li, 2021-01-11 11:28 AM

Related issues

Related to CUNY Academic Commons - Support #13637: All-in-One Migration plugin requestAbandoned2020-12-01

History

#1 Updated by Alan Li 12 months ago

Hello,

To answer the questions from your response,

1) The target site on the Commons would be the LMIS one. https://lmis.commons.gc.cuny.edu/
2) I have access to the .sql file for the database. Is this the file that you were talking about?
3) I am not using any plugins at all aside from the All-in-One Migration that I can delete.
4) I am creating my own theme and templates to use for the website.
5) I do not know exactly what this means, sorry about that. Does this mean who will have access to edit the files, or does it mean the directed audience (for example: only "X" users can see such and such and all users can see "Y")?

Thank you for all the help,
Alan Li

#2 Updated by Boone Gorges 12 months ago

  • Status changed from New to Reporter Feedback

Thanks for chiming in, Alan!

A couple of my points (2 and 5) assume that your local version of the site has some content that needs to be migrated to the production site. "Content" here means anything stored in the database: Posts and pages; items in the media library (if these are present then I'll also need a copy of your wp-content/uploads directory); also theme configuration, site settings, and other such data. If all you're doing locally is developing a theme - without content, or with minimal content/settings that can be manually configured on the Commons - it makes this process much, much easier. So maybe you could say a bit more about whether you need content migrated en masse?

4) I am creating my own theme and templates to use for the website.

Can you share this with me, either as a zip or in the form of a GitHub link, so that I can begin review? See https://dev.commons.gc.cuny.edu/hosting-partner-handbook/#custom-themes-and-plugins

#3 Updated by Marilyn Weber 12 months ago

And here's a reply from Heather Sutton:

Thank you, Marilyn and Boone! The GC emails for our team who will be admins on the site are and

Best,
Heather

#4 Updated by Alan Li 12 months ago

Sorry for the extremely late response. I have been super busy with my group project and got carried away by it. This is my first WordPress experience, so correct me if I am wrong at any time. I believe that I am just creating theme using WordPress, but I believe all of the content is inside. I attached the whole folder that has the content of the LMIS barring some notes and stuff -- from what I understand, I think I do need to migrate the content as a whole. I am new to WordPress and coding in PHP in general, so if there's a more optimal way to format it so that it will be easier for you, then let me know and I can make adjustments.

Thank you so much,
Alan Li

#5 Updated by Boone Gorges 12 months ago

Thanks, Alan. I'm happy to provide recommendations on WordPress best practices or answers to any questions you might have.

I've had a chance to look through the zip file that you sent and I have a bit of feedback.

First, a general note about our policies and procedures for custom themes. You can read more details at https://dev.commons.gc.cuny.edu/hosting-partner-handbook/, but an important point for this project is that all custom code has to come through me and the development team, and it will be released to the production website only after review, and only during regularly scheduled bi-monthly maintenance releases.

This makes it difficult to do quick iterations on your work. So, for example, it appears that you've hardcoded lots of content into templates, especially in the case of page.php. It's OK with me if you do this, but note that it will therefore not be editable by the site admin, and will only be editable through the procedures for updating custom code. This kind of turnaround might not be ideal for your team. In contrast, if this content were stored inside of WordPress's posts, your PHP template could load the content from the database and display it (the_content();) while remaining easily editable by members of the team.

Similarly, I see that some parts of the theme seem to be unfinished. For example, there's a section on index.php that contains some placeholder text ("Insert some text here about the subject"). If we release this as-is, there will be no way to fix it until the following maintenance release. So we need to be sure that the theme is ready to go before putting it into action, because quick edits will not be possible.

In addition to this general comment, there are a few conventions of WordPress development that need to be adhered to, or the theme won't work in the Commons environment:
- Perhaps most importantly, you must be sure that URL paths are built dynamically. In some places you have hardcoded paths like http://cunygraduatelmis.local, which obviously won't work in production. There are lots of tools in WordPress to build dynamic links, but for your purpose I recommend the following: Anywhere you need to reference a path that's inside the theme directory, use get_stylesheet_directory_uri(), and anytime you need to reference a path relative to the site URL, use home_url(). Examples:


<img class="logo" src="<?php echo esc_attr( get_stylesheet_directory_uri() ); ?>/images/thumbnail.png"> // instead of hardcoded local path

<a href="<?php echo esc_attr( home_url( 'resources' ) );"> RESOURCES </a> // instead of "/resources" 

esc_attr() in these examples is a security precaution against XSS. See https://developer.wordpress.org/themes/basics/linking-theme-files-directories/, https://developer.wordpress.org/themes/theme-security/data-sanitization-escaping/

- It's important to load the local version of jQuery instead of one from a CDN, or it'll cause conflicts with the rest of the Commons JavaScript. This means enqueuing in functions.php rather than a hardcoded <link>, and probably also means that you need to enqueue other scripts as well, to ensure the proper dependency relationships. See https://developer.wordpress.org/themes/basics/including-css-javascript/

As for the rest of the migration, it looks like you have a very small number of pages, so it's not really necessary to do a full SQL import. When the time comes, I recommend that you use the built-in WP export/import tools to move your content over. See https://help.commons.gc.cuny.edu/importing-and-exporting-site-content/. The main part of the migration will be the theme handoff, so in the future it's OK to give me a zip only of the wp-content/themes/cuny-graduate-lmis directory.

#6 Updated by Alan Li 12 months ago

Hi Mr. Gorges,

Thank you so much for the in-depth code review and the helpful links to help me understand this process. I understand what I have to change, and I will try to implement the changes you suggested in this post. I will try to have it done within the next few days, since I am busy with schoolwork at the moment (finals and projects). Will it be fine to resend the updated version for you to check whenever these changes are fixed? This version will not be the complete one, but hopefully, it will be able to load links and content dynamically -- I was working on this assignment during the summer, and I agreed to work on it intermittently during the semester. However, I got bombarded with a lot of work, so I could not get to it, and LMIS wants to have their website done by the end of December ideally. The template that I will be sending in the end will not be finalized, but it should be good enough to load all the current content for the next person to work on it.

Best,
Alan Li

#7 Updated by Boone Gorges 12 months ago

Thanks, Alan. There's no rush from my end. Please just upload a zip file to this ticket (or provide a link to a public repository) so that I can review the theme changes.

#8 Updated by Boone Gorges 12 months ago

  • Related to Support #13637: All-in-One Migration plugin request added

#9 Updated by Alan Li 12 months ago

Hi Mr. Gorges,

I hope you are having a good day today. I just finished my finals for this semester, so I will be finalizing the changes to my code; I am so sorry for the huge delay. I intend to finish working on the code tonight or by tomorrow. Is it still fine to send you the code to review? If so, when are you available in the next week or so since the holidays are coming up? I am available for the entirety of December, so I can respond as soon as possible if there are any issues regarding the code.

Best,
Alan Li

#10 Updated by Boone Gorges 12 months ago

Hi Alan - I'll be available to review early next week if you can have something to me this weekend. We have a regularly scheduled release on Dec 22 and it would be nice to get this in by then, if we can complete our back-and-forth by that point.

#11 Updated by Alan Li 12 months ago

Hi Mr. Gorges,

Attached is the updated version of the code I sent a while ago. This took longer than I expected because I kept switching methods to load the content for each page. I understood what you meant by creating posts and loading the content based on that. I was able to accomplish this in the About page because it did not have a lot of nested information and was relatively easy to style. However, I cannot seem to figure out how to style more complex pages with more content and styling.

As an example, in the Impact page (Partners->Impact) (ignore the alignment), would I create a post for each card? How would I be able to create divs with particular classes to create this effect? Additionally, there's an anchor tag on this page that opens a pdf in another window; how can this be done by writing all the content in the page itself? I have been looking up and trying to solve this question for the last two days. I understand that it is not necessary to do this implementation using the_content(), but I still want to try it because in addition to making it easier to editors, I also want to comprehend the structure/format that developers use for the sake of learning.

I changed all of the hardcoded paths to the method you provided in your response; is this fine now?

I also took out the jQuery CDN, and I did wp_enqueue_script('jquery') in the functions.php instead. Afterwards, I loaded up my own script in the same function. Will this cause an issue in the future where the jQuery will somehow load slower than my personal jQuery script? Additionally, you mentioned specifically to change the jQuery CDN, so I did not change the Bootstrap one at all. Just to double-check, should I also place the Bootstrap script and link inside functions.php?

As an aside, the outcome of the code review is to be able to "migrate" the current progress into CUNY Academic Commons; I will not be finishing up the project, so it has to be usable by the next person. Sorry for the lengthy response, and I am super grateful for your help. I came into the project with no knowledge in PHP at all, so I apologize once again for my lack of understanding and messy code.

Best,
Alan Li

#12 Updated by Boone Gorges 12 months ago

Thanks for the update, Alan.

As an example, in the Impact page (Partners->Impact) (ignore the alignment), would I create a post for each card? How would I be able to create divs with particular classes to create this effect? Additionally, there's an anchor tag on this page that opens a pdf in another window; how can this be done by writing all the content in the page itself? I have been looking up and trying to solve this question for the last two days. I understand that it is not necessary to do this implementation using the_content(), but I still want to try it because in addition to making it easier to editors, I also want to comprehend the structure/format that developers use for the sake of learning.

There are various ways to accomplish this. One is through the use of custom blocks. See eg https://deliciousbrains.com/custom-gutenberg-block/. But this may be a lot of work if you don't have a background in the necessary technologies. Another way is by leveraging WordPress shortcodes, which can be a fairly user-friendly way of creating specific kinds of markup. See eg https://developer.wordpress.org/plugins/shortcodes/ You could also try simply building the content in the WordPress editor as close as reasonably possible, and then using the default classes (along with perhaps some of the specific body classes that WP adds to its markup, to differentiate between pages) to target elements, rather than using custom classes in the markup.

If none of these routes seems viable, it's probably fine to stick with page templates. But they should be defined and loaded in the correct way. Page Templates can be associated with specific pages if they are defined properly; see https://developer.wordpress.org/themes/template-files-section/page-template-files/. Alternatively, you can specify a template file to be used with a specific page using the naming conventions in the WP Template Hierarchy. See https://developer.wordpress.org/themes/basics/template-hierarchy/

I changed all of the hardcoded paths to the method you provided in your response; is this fine now?

This looks good in some places, but searching the theme for '.local' I still see many instances.

I also took out the jQuery CDN, and I did wp_enqueue_script('jquery') in the functions.php instead. Afterwards, I loaded up my own script in the same function. Will this cause an issue in the future where the jQuery will somehow load slower than my personal jQuery script? Additionally, you mentioned specifically to change the jQuery CDN, so I did not change the Bootstrap one at all. Just to double-check, should I also place the Bootstrap script and link inside functions.php?

I still see that jQuery is being loaded via script tag at the top of index.php. Using the CAC version is the best way to ensure compatibility going forward, and it should not cause performance issues. It is OK to load Bootstrap from the CDN as you've done.

As an aside, the outcome of the code review is to be able to "migrate" the current progress into CUNY Academic Commons; I will not be finishing up the project, so it has to be usable by the next person. Sorry for the lengthy response, and I am super grateful for your help. I came into the project with no knowledge in PHP at all, so I apologize once again for my lack of understanding and messy code.

Understood. No apologies are necessary - the requirements of the Commons means that the process is more complex than it might otherwise be. Thanks for your patience working through these issues.

#13 Updated by Alan Li 12 months ago

Thank you for the prompt response Mr. Gorges,

I will look into custom blocks and shortcodes to see if I can utilize either of them to implement the changes necessary. I am not 100% confident that I can do it, but I will attempt it, since it will be easier for others in the future.

Regarding the routes, I can change the ones with no route to have a page template; this seems simple and practical to accomplish. I will take a look at the documentation to comprehend the material better as well.

A lot of the .local that I see are commented out aside from the one in the home page, which I will convert now. I can get rid of all instances of '.local' indefinitely by the next iteration of the code.

I forgot to remove the script tag for jQuery at the top of index.php; if I take it out and just use the one in functions.php, does this mean I am using the CAC version?

I sincerely thank you for being so patient and thorough with the code review -- I have learned a lot during this migration process. I will be attempting to use custom blocks or shortcodes today so that I can have another updated version by tomorrow. I understand that you might be busy tomorrow, so let me know when and if it is possible for you to look over the hopefully final iteration of the code in the future.

Best,
Alan Li

#14 Updated by Alan Li 11 months ago

Sorry for the long, long delay,

I hope you had a nice holiday. I had a lot of technical difficulties and just fixed it recently by buying a new part for my computer.

As for the code, I decided to use the shortcode method that you mentioned, and I was able to make it so that the users can edit easily.

If it is possible, could you review my code to see if there are any conflicts?

Best,
Alan Li

#15 Updated by Boone Gorges 11 months ago

  • Target version set to 1.18.2

Thanks, Alan.

It looks like you've addressed my most critical concerns. I've added the theme to the codebase in https://github.com/cuny-academic-commons/cac/commit/227cf7e9ca715cfb6b6cdcbf29728b691f1e2c4b and it will be available after tomorrow's scheduled release. I'll update this ticket when it's available for activation on lmis.commons.gc.cuny.edu.

Your changes to use the_content() and shortcodes for the theme are generally good, definitely improvements over the prior version I saw. There's still room for significant streamlining. For example, most of your theme templates (about.php, impact.php, etc) have pretty much the same content. This suggests that you could have a single page.php with this content, which would mean that you don't have to go to the trouble of setting the Page Template when creating the page (it'll use page.php as the default/fallback).

Similarly, you've got lots of separate methods for shortcodes, but they are nearly identical. So, for example, instead of separate jaclyn(), anindya etc methods, you could have a single lmis_staff_profile() shortcode handler, and you could pass the profile image URL in as a shortcode attribute/function parameter, as you do with 'position', 'location', etc. This would dramatically decrease the amount of custom code in your theme.

I'll mark this ticket Resolved after tomorrow's release. If you make more updates in the future, please open a new ticket.

#16 Updated by Boone Gorges 11 months ago

  • Status changed from Reporter Feedback to Resolved

The theme is now available to activate on lmis.commons.gc.cuny.edu.

Also available in: Atom PDF