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

-- webtest cleanup (used openCiviPage function in place of open) #4

Merged
merged 1 commit into from
Mar 1, 2013
Merged

-- webtest cleanup (used openCiviPage function in place of open) #4

merged 1 commit into from
Mar 1, 2013

Conversation

ravishnair
Copy link
Contributor

Replaced open function with the openCiviPage function.

kurund added a commit that referenced this pull request Mar 1, 2013
-- webtest cleanup (used openCiviPage function in place of open)
@kurund kurund merged commit 59535ee into civicrm:master Mar 1, 2013
@colemanw
Copy link
Member

colemanw commented Mar 1, 2013

Note: The waitForPageToLoad() method should not be called after openCiviPage since it will be redundant.
The first line of your patch there is one left in there.

MegaphoneJon pushed a commit to MegaphoneJon/civicrm-core that referenced this pull request Apr 30, 2013
worked on CRM-12096, set component title, that fixes notice in j3
colemanw pushed a commit to twomice/civicrm-core that referenced this pull request Apr 30, 2013
@ravishnair ravishnair mentioned this pull request Jun 14, 2013
kurund added a commit to kurund/civicrm-core that referenced this pull request Jul 12, 2013
rohankatkar pushed a commit to rohankatkar/civicrm-core-master that referenced this pull request May 22, 2014
VAT-86-94-224 Added validation when 'Is Tax?' is checked. Checked for fi...
kurund added a commit that referenced this pull request Dec 23, 2014
eileenmcnaughton referenced this pull request in eileenmcnaughton/civicrm-core Jan 6, 2015
Edzelopez added a commit to Edzelopez/civicrm-core that referenced this pull request Mar 16, 2015
MegaphoneJon pushed a commit to MegaphoneJon/civicrm-core that referenced this pull request Apr 26, 2015
alias level as severity for Status paradigm; CRM-13823;
kurund pushed a commit to kurund/civicrm-core that referenced this pull request May 12, 2015
@KarinG KarinG mentioned this pull request Aug 7, 2016
f2boot pushed a commit to f2boot/civicrm-core that referenced this pull request Sep 25, 2016
ErichBSchulz pushed a commit to ErichBSchulz/civicrm-core that referenced this pull request Jan 3, 2017
CRM-16313. Do not reject IDN email addresses.
colemanw pushed a commit that referenced this pull request Apr 1, 2017
* CRM-20345 - CRM_Utils_Array::crmArraySortByField - Add test. Allow multiple fields.

* CRM-20345 - CRM_Utils_SQL_Select::orderBy - Use more deterministic ordering

The technique of computing default `$weight = count($this->orderBys)`
addresses a valid point: we need to preserve ordering for existing callers
who don't specify weights -- while also allowing weights.

However, it feels weird in my gut. Not sure why -- maybe it's something like this:

```php
// A1: Non-deterministic ordering
$select->orderBy('alpha', 1);
$select->orderBy('beta');
$select->orderBy('delta', 2);
$select->orderBy('gamma', 3);

// A2: Deterministic ordering
$select->orderBy('alpha', 10);
$select->orderBy('beta');
$select->orderBy('delta', 20);
$select->orderBy('gamma', 30);

// B1: Deterministic ordering
$select->orderBy('alpha');
$select->orderBy('beta');
$select->orderBy('delta');
$select->orderBy('gamma');

// B2: Non-deterministic ordering
$select->orderBy('alpha', 1);
$select->orderBy('beta', 1);
$select->orderBy('delta', 1);
$select->orderBy('gamma', 1);
```

As a reader, I would expect A1/A2 to be the same, and I would expect B1/B2
to be the same.  But they're not.  If there's a collision in the `weight`s,
the ordering becomes non-deterministic (depending on obscure details or
happenstance of the PHP runtime).

Of course, there's no right answer: in A1/A2, you can plausibly put `beta`
before `alpha` or after `alpha` or after `gamma`.  But it should be
determinstic so that it always winds up in the same place.
JohnFF referenced this pull request Apr 20, 2017
The recent revision #9595 updated `ExternalBatch` to produce a more
informative error message.  However, in the test environment used by
`flexmailer`, there was exactly 1 value in `$_ENV` (even if
`variables_order` was misconfigured).

This should make the test a bit harder to trip-up by affirmatively checking
for the most common environment variable.

Review note: In the known universe, the only direct references to
`ExternalBatch` are in `CiviUnitTestCase`, so this shouldn't impact any
runtime logic.
totten pushed a commit that referenced this pull request Jun 16, 2017
CRM-20561 - Remove example files. Cleanup code style.
totten added a commit to totten/civicrm-core that referenced this pull request Jul 28, 2017
There appears to be some application logic which follows a process like this:

 1. Read the case
 2. Tweak the data
 3. Save updated case

The problem is comes if step civicrm#4 resaves a timestamp loaded in step #1, which
is fairly likely to happen if you read+save the same record.

This was specifically observed on the "Manage Case" screen when editing
activities, but the data-flow is pretty common, so make a general fix to the
BAO.
totten added a commit to totten/civicrm-core that referenced this pull request Aug 1, 2017
There appears to be some application logic which follows a process like this:

 1. Read the case
 2. Tweak the data
 3. Save updated case

The problem is comes if step civicrm#4 resaves a timestamp loaded in step #1, which
is fairly likely to happen if you read+save the same record.

This was specifically observed on the "Manage Case" screen when editing
activities, but the data-flow is pretty common, so make a general fix to the
BAO.
totten added a commit to totten/civicrm-core that referenced this pull request Aug 2, 2017
There appears to be some application logic which follows a process like this:

 1. Read the case
 2. Tweak the data
 3. Save updated case

The problem is comes if step civicrm#4 resaves a timestamp loaded in step #1, which
is fairly likely to happen if you read+save the same record.

This was specifically observed on the "Manage Case" screen when editing
activities, but the data-flow is pretty common, so make a general fix to the
BAO.
@eileenmcnaughton eileenmcnaughton mentioned this pull request Sep 19, 2021
totten added a commit to totten/civicrm-core that referenced this pull request Oct 8, 2021
Use-case:

1. Open the editor a message template (eg `#/edit?id=17&lang=fr_FR`). This renders the edit screen.
2. Click on the "Preview" icon.
3. Close the "Preview" dialog.
4. Change the URL to navigate internally to another template (eg `#/edit?id=17&lang=de_DE`). This renders the edit screen again.
5. Click on the "Preview" icon.

Before:

* It subsequently opens two copies of the "Preview" dialog (each with
  slightly different options).  Initially, you see one dialog.  When that
  close, you will see the other dialog.
* As you proceed through closing the dialogs, you may get console warnings
  because the dialogs have the same name -- and they consequently trip-up
  each other.

After:

* It only opens one copy of the "Preview" dialog.

Technical Details:

* When rendering the setup screen (`#1`/`civicrm#4`), it registers a listener which
  will handle the "Preview" clicks.  But the listener is not properly
  unregistered.  Consequently, old listeners hang around. So the click at
  step `civicrm#5` calls both the old+new listeners, creating two dialogs.
totten added a commit to totten/civicrm-core that referenced this pull request Oct 8, 2021
Use-case:

1. Open the editor a message template (eg `#/edit?id=17&lang=fr_FR`). This renders the edit screen.
2. Click on the "Preview" icon.
3. Close the "Preview" dialog.
4. Change the URL to navigate internally to another template (eg `#/edit?id=17&lang=de_DE`). This renders the edit screen again.
5. Click on the "Preview" icon.

Before:

* It subsequently opens two copies of the "Preview" dialog (each with
  slightly different options).  Initially, you see one dialog.  When that
  close, you will see the other dialog.
* As you proceed through closing the dialogs, you may get console warnings
  because the dialogs have the same name -- and they consequently trip-up
  each other.

After:

* It only opens one copy of the "Preview" dialog.

Technical Details:

* When rendering the setup screen (`#1`/`civicrm#4`), it registers a listener which
  will handle the "Preview" clicks.  But the listener is not properly
  unregistered.  Consequently, old listeners hang around. So the click at
  step `civicrm#5` calls both the old+new listeners, creating two dialogs.
totten added a commit to totten/civicrm-core that referenced this pull request Nov 30, 2021
…ion-fix)

Overview
--------

Fixes a recent regression that prevents you from uninstalling extensions via
CLI.  This specifically affects extensions which use managed entities.

Steps to reproduce
------------------

```
cv en afform
cv dis afform
cv ext:uninstall afform
```

Before
-------

```
[bknix-max:~/bknix/build/dmaster/web/sites/all/modules/civicrm] cv en afform && cv dis afform && cv ext:uninstall afform
Enabling extension "org.civicrm.afform"
Disabling extension "org.civicrm.afform"
Uninstalling extension "org.civicrm.afform"
Error: API Call Failed: Array
(
    [entity] => Extension
    [action] => uninstall
    [params] => Array
        (
            [keys] => Array
                (
                    [0] => org.civicrm.afform
                )

            [debug] => 1
            [version] => 3
        )

    [result] => Array
        (
            [error_code] => unauthorized
            [entity] => Extension
            [action] => uninstall
            [trace] => #0 /home/me/bknix/build/dmaster/web/sites/all/modules/civicrm/Civi/API/Kernel.php(147): Civi\API\Kernel->authorize(Object(Civi\Api4\Provider\ActionObjectProvider), Object(Civi\Api4\Generic\DAODeleteAction))
 #1 /home/me/bknix/build/dmaster/web/sites/all/modules/civicrm/Civi/Api4/Generic/AbstractAction.php(234): Civi\API\Kernel->runRequest(Object(Civi\Api4\Generic\DAODeleteAction))
 #2 /home/me/bknix/build/dmaster/web/sites/all/modules/civicrm/api/api.php(85): Civi\Api4\Generic\AbstractAction->execute()
 civicrm#3 /home/me/bknix/build/dmaster/web/sites/all/modules/civicrm/CRM/Core/ManagedEntities.php(467): civicrm_api4('OptionValue', 'delete', Array)
 civicrm#4 /home/me/bknix/build/dmaster/web/sites/all/modules/civicrm/CRM/Core/ManagedEntities.php(303): CRM_Core_ManagedEntities->removeStaleEntity(Object(CRM_Core_DAO_Managed))
 civicrm#5 /home/me/bknix/build/dmaster/web/sites/all/modules/civicrm/CRM/Core/ManagedEntities.php(134): CRM_Core_ManagedEntities->reconcileUnknownModules()
 civicrm#6 /home/me/bknix/build/dmaster/web/sites/all/modules/civicrm/CRM/Core/Invoke.php(409): CRM_Core_ManagedEntities->reconcile()
 civicrm#7 /home/me/bknix/build/dmaster/web/sites/all/modules/civicrm/CRM/Extension/Manager.php(483): CRM_Core_Invoke::rebuildMenuAndCaches(true)
 civicrm#8 /home/me/bknix/build/dmaster/web/sites/all/modules/civicrm/api/v3/Extension.php(183): CRM_Extension_Manager->uninstall(Array)
 civicrm#9 /home/me/bknix/build/dmaster/web/sites/all/modules/civicrm/Civi/API/Provider/MagicFunctionProvider.php(89): civicrm_api3_extension_uninstall(Array)
 civicrm#10 /home/me/bknix/build/dmaster/web/sites/all/modules/civicrm/Civi/API/Kernel.php(149): Civi\API\Provider\MagicFunctionProvider->invoke(Array)
 civicrm#11 /home/me/bknix/build/dmaster/web/sites/all/modules/civicrm/Civi/API/Kernel.php(81): Civi\API\Kernel->runRequest(Array)
 civicrm#12 /home/me/bknix/build/dmaster/web/sites/all/modules/civicrm/api/api.php(22): Civi\API\Kernel->runSafe('Extension', 'uninstall', Array)
 civicrm#13 phar:///home/me/bknix/bin/cv/src/Command/BaseCommand.php(49): civicrm_api('Extension', 'uninstall', Array)
 civicrm#14 phar:///home/me/bknix/bin/cv/src/Command/ExtensionUninstallCommand.php(63): Civi\Cv\Command\BaseCommand->callApiSuccess(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput), 'Extension', 'uninstall', Array)
 civicrm#15 phar:///home/me/bknix/bin/cv/vendor/symfony/console/Command/Command.php(257): Civi\Cv\Command\ExtensionUninstallCommand->execute(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
 civicrm#16 phar:///home/me/bknix/bin/cv/vendor/symfony/console/Application.php(850): Symfony\Component\Console\Command\Command->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
 civicrm#17 phar:///home/me/bknix/bin/cv/vendor/symfony/console/Application.php(193): Symfony\Component\Console\Application->doRunCommand(Object(Civi\Cv\Command\ExtensionUninstallCommand), Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
 civicrm#18 phar:///home/me/bknix/bin/cv/src/Application.php(46): Symfony\Component\Console\Application->doRun(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
 civicrm#19 phar:///home/me/bknix/bin/cv/vendor/symfony/console/Application.php(124): Civi\Cv\Application->doRun(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
 civicrm#20 phar:///home/me/bknix/bin/cv/src/Application.php(15): Symfony\Component\Console\Application->run()
 civicrm#21 phar:///home/me/bknix/bin/cv/bin/cv(27): Civi\Cv\Application::main('phar:///Users/t...')
 civicrm#22 /home/me/bknix/bin/cv(14): require('phar:///Users/t...')
 civicrm#23 {main}
            [is_error] => 1
            [error_message] => Authorization failed
        )

)
```

After
-----

Works

Comment
-------

I encountered this while working on E2E test-coverage for other changes.
The E2E test coverage had worked on a previous iteration of 5.45.alpha1 but
failed when I rebased. Consequently, this means

You can see a prior draft of the E2E test [here](https://github.com/totten/shimmy/blob/master-reorg/shimmy/tests/phpunit/E2E/Shimmy/LifecycleTest.php#L56-L77).
However, it's being reworked as a core patch.

I'd suggest accepting this without a test - because (a) it's a regression and (b) there will be coverage from the pending change.
totten pushed a commit that referenced this pull request Sep 5, 2024
CRM_Event_Page_EventInfoTest::testFullMessage
Exception: CRM_Extension_Exception_MissingException: "Failed to find extension: civi_mail"
#0 /home/homer/buildkit/build/build-3/web/sites/all/modules/civicrm/CRM/Extension/Container/Basic.php(143): CRM_Extension_Container_Basic->getRelPath("civi_mail")
#1 /home/homer/buildkit/build/build-3/web/sites/all/modules/civicrm/CRM/Extension/Mapper.php(233): CRM_Extension_Container_Basic->getPath("civi_mail")
#2 /home/homer/buildkit/build/build-3/web/sites/all/modules/civicrm/CRM/Core/Resources.php(261): CRM_Extension_Mapper->keyToBasePath("civi_mail")
#3 /home/homer/buildkit/build/build-3/web/sites/all/modules/civicrm/CRM/Core/Resources.php(311): CRM_Core_Resources->getPath("civi_mail")
#4 /home/homer/buildkit/build/build-3/web/sites/all/modules/civicrm/Civi/Angular/Manager.php(208): CRM_Core_Resources->glob("civi_mail", (Array:3))
#5 /home/homer/buildkit/build/build-3/web/sites/all/modules/civicrm/Civi/Angular/Manager.php(114): Civi\Angular\Manager->resolvePatterns((Array:59))
#6 /home/homer/buildkit/build/build-3/web/sites/all/modules/civicrm/CRM/Utils/Check/Component/Env.php(1178): Civi\Angular\Manager->getModules()
#7 /home/homer/buildkit/build/build-3/web/sites/all/modules/civicrm/CRM/Utils/Check/Component.php(76): CRM_Utils_Check_Component_Env->checkAngularModuleSettings(FALSE)
#8 /home/homer/buildkit/build/build-3/web/sites/all/modules/civicrm/CRM/Utils/Check.php(215): CRM_Utils_Check_Component->checkAll((Array:0), FALSE)
#9 /home/homer/buildkit/build/build-3/web/sites/all/modules/civicrm/CRM/Utils/Check.php(185): CRM_Utils_Check::checkStatus()
#10 /home/homer/buildkit/build/build-3/web/sites/all/modules/civicrm/CRM/Utils/Check.php(93): CRM_Utils_Check::checkAll()
#11 /home/homer/buildkit/build/build-3/web/sites/all/modules/civicrm/CRM/Core/Page.php(267): CRM_Utils_Check->showPeriodicAlerts()
#12 /home/homer/buildkit/build/build-3/web/sites/all/modules/civicrm/CRM/Event/Page/EventInfo.php(325): CRM_Core_Page->run()
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.

3 participants