Page MenuHomeIn-Portal Phabricator
Differential D503

INP-1893 Denormalize "ChangeLogs" database table
AcceptedPublic
Actions

Authored by erik on Wed, Jan 22, 12:20 PM.

Details

Reviewers
alex
Test Plan

Preparations

  1. Install previous version (522-B1)
  2. Login to adm.console as root
  3. Go to the Logs & Reports → Changes Log section
  4. Enable Changes Log by using special link above grid
  5. Logout
  6. Login to adm.console as root
  7. Go to the User Management → Administrators section
  8. Create administrative user, fill First Name, Last Name and Email fields
  9. Logout
  10. Login to adm.console as non-root
  11. Go to the User Management → Administrators section
  12. Edit administrative user, change some field value, save changes

Part 1 - upgrade test

  1. Make In-Portal Upgrade
  2. Login to adm.console
  3. Go to the Logs & Reports → Changes Log section
  4. Confirm, that Username, First Name, Last Name, Email columns contains proper data.
  5. Open debugger panel. Confirm, that no joins in sql request to ChangeLogs table.

Part 2 - clean install test

  1. Make In-Portal Clean install
  2. Login to adm.console as root
  3. Go to the Logs & Reports → Changes Log section
  4. Enable Changes Log by using special link above grid
  5. Logout
  6. Login to adm.console as root
  7. Go to the User Management → Administrators section
  8. Create administrative user, fill First Name, Last Name and Email fields
  9. Logout
  10. Login to adm.console as non-root
  11. Go to the User Management → Administrators section
  12. Edit administrative user, change some field value, save changes
  13. Go to the Logs & Reports → Changes Log section
  14. Confirm, that Username, First Name, Last Name, Email columns contains proper data.
  15. Open debugger panel. Confirm, that no joins in sql request to ChangeLogs table.

Diff Detail

Repository
rINP In-Portal
Branch
/in-portal/branches/5.2.x
Lint
Lint ErrorsExcuse: all code style fixes is not part of this task
SeverityLocationCodeMessage
Errorcore/units/logs/change_logs/change_logs_config.php:116PHPCS.E.CodingStandard.Arrays.Array.SpaceAfterKeywordCodingStandard.Arrays.Array.SpaceAfterKeyword
Errorcore/units/logs/change_logs/change_logs_config.php:116PHPCS.E.Generic.PHP.LowerCaseKeyword.FoundGeneric.PHP.LowerCaseKeyword.Found
Unit
No Unit Test Coverage
Build Status
Buildable 11051
Build 3751: arc lint + arc unit

Event Timeline

erik created this revision.Wed, Jan 22, 12:20 PM
erik requested review of this revision.Wed, Jan 22, 12:20 PM
erik edited the test plan for this revision. (Show Details)Wed, Jan 22, 12:21 PM
alex requested changes to this revision.Wed, Jan 22, 12:39 PM

I haven't executed the test plan yet. Please make the proposed corrections/answer questions.

core/install/install_schema.sql
691–694

Please move the declaration of these columns after PortalUserId column.

core/install/upgrades.php
2558–2565 ↗(On Diff #1303)

Please explain pros/cons of this implementation compared to executing this SQL in the upgrades.sql file?

core/install/upgrades.sql
2990–2993

Please move the declaration of these columns after PortalUserId column.

2994

Is it possible for Guest user to cause change log record creation?

If so, then please also handle this across the Differential Revision.

core/kernel/db/db_event_handler.php
2107–2109

Please remove.

No need to assign empty values, because they match default values in these columns.

2111

Please replace with:

$cache[$user_id] += $user_data;

Array concatenation would work faster in this case.

2122–2131

Please replace with:

if ( $user_data !== false ) {
    $cache[$user_id] += $user_data;
}
  • Array concatenation would work faster in this case.
  • No need to assign empty values, because they match default values in these columns.
This revision now requires changes to proceed.Wed, Jan 22, 12:39 PM
erik marked an inline comment as done.Thu, Jan 23, 4:54 AM
erik added inline comments.
core/install/upgrades.php
2558–2565 ↗(On Diff #1303)

Not working in upgrades.sql, because no table prefix replacement for joining table Users.

erik updated this revision to Diff 1304.Thu, Jan 23, 5:19 AM
erik marked 6 inline comments as done.
erik edited the test plan for this revision. (Show Details)

Done all requested improvements

erik added inline comments.Thu, Jan 23, 9:27 AM
core/install/upgrades.sql
2994

Not possible change log for guest user, because session variable '_SessionLogId_' is required for change log logging. And this variable is set only after login.

alex requested changes to this revision.Thu, Jan 23, 10:32 AM

The test passed. However, please make these minor adjustments.

core/install/upgrades.php
2558–2565 ↗(On Diff #1303)

For such cases use <%TABLE_PREFIX%> placeholder as done in the below example (from the upgrades.sql file):

INSERT INTO Permissions (Permission, GroupId, PermissionValue, Type, CatId)
SELECT 'in-portal:configemail.add' AS Permission, GroupId, PermissionValue, Type, CatId
FROM <%TABLE_PREFIX%>Permissions
WHERE Permission = 'in-portal:configemail.edit';
core/kernel/db/db_event_handler.php
2085

Please correct a typo in the method parameter description.

2114

Please only use TABs for indentation.

This revision now requires changes to proceed.Thu, Jan 23, 10:32 AM
erik updated this revision to Diff 1305.Thu, Jan 23, 10:45 AM

Done all requested minor fixes

alex accepted this revision.Fri, Jan 24, 3:10 AM
This revision is now accepted and ready to land.Fri, Jan 24, 3:10 AM
alex updated this revision to Diff 1306.Fri, Jan 24, 3:11 AM

Minor SQL formatting changes.

alex retitled this revision from INP-1893 Don't use JOIN's in "Change Logs" grid to INP-1893 Denormalize "ChangeLogs" database table.Sun, Jan 26, 1:57 PM

Revision Contents
Changeset List

DiffIDBaseDescriptionCreatedLintUnit
BaseBase
Diff 1130316823Wed, Jan 22, 12:20 PM
★
★
Diff 2130416823Done all requested improvementsThu, Jan 23, 5:19 AM
★
★
Diff 3130516823Done all requested minor fixesThu, Jan 23, 10:45 AM
★
★
Diff 4130616823Minor SQL formatting changes.Fri, Jan 24, 3:11 AM
★
★

Diff 1305

View Options

core/install/install_schema.sql

Loading...
View Options

core/install/upgrades.sql

Loading...
View Options

core/kernel/db/db_event_handler.php

Loading...
View Options

core/units/logs/change_logs/change_logs_config.php

Loading...
In-Portal CMS · In-Portal Community Pages · In-Portal Issue Tracker · In-Portal Repository Browser · Quality Assurance · In-Portal CI Server