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

Remove comments system for tags - really not useful. #11

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

Conversation

fiammybe
Copy link
Contributor

closes #5

@fiammybe fiammybe added this to the 1.2 milestone Jul 27, 2024
@fiammybe fiammybe self-assigned this Jul 27, 2024
Copy link
Contributor

@skenow skenow left a comment

Choose a reason for hiding this comment

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

I don't see how any comments could have been added to a tag or category, so no need to worry about orphaned records in the database.

There are some additional references to comments - here's one in tag.php
if ($icmsModuleConfig['com_rule'] && $tagObj->getVar('tag_cancomment')) {
$icmsTpl->assign('imtagging_tag_comment', true);
include_once ICMS_ROOT_PATH . '/include/comment_view.php';
}

@fiammybe fiammybe requested a review from skenow July 30, 2024 08:19
Copy link
Contributor

@skenow skenow left a comment

Choose a reason for hiding this comment

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

Check the upgradeObjectItem method, it may simplify the update code.

There are also 2 config items that will need to be deleted - the com_rule and com_anonpost for this module. The rest of this looks good!

function imtagging_db_upgrade_2() {
$icmsDatabaseUpdater = icms_db_legacy_Factory::getDatabaseUpdater();
//$icmsDatabaseUpdater->;
$sql = sprintf("ALTER TABLE %s DROP column tag_cancomment", icms::$xoopsDB->prefix('imtagging_tag'));
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if the upgrade method handles this without having to explicitly do this. See icms_db_legacy_updater_Handler::upgradeObjectItem().

Copy link
Contributor

Choose a reason for hiding this comment

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

It does - I just removed the 2 lines in class/tag.php for tag_cancomment and updated the module. That was enough to drop the column.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great! that makes version updates considerably easier, at least when it comes to removing fields.

'title' => '_MI_IMTAGGING_LIMIT',
'description' => '_MI_IMTAGGING_LIMITDSC',
'title' => _MI_IMTAGGING_LIMIT,
'description' => _MI_IMTAGGING_LIMITDSC,
'formtype' => 'textbox',
'valuetype' => 'text',
'default' => 5);

Copy link
Contributor

Choose a reason for hiding this comment

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

When I added config items to imBlogging, the language constants did need to be enclosed in quotes to be treated as a constant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that is correct, error on my part.

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

Successfully merging this pull request may close these issues.

Remove comment settings
2 participants