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

Multi AOP #5426

Closed
wants to merge 29 commits into from
Closed

Multi AOP #5426

wants to merge 29 commits into from

Conversation

gymad
Copy link
Contributor

@gymad gymad commented Mar 7, 2018

Description

ability to join multiple joomla site to suitecrm

Motivation and Context

How To Test This

join multiple joomla site to crm, try each functionality in cases module with multiple users...
test for backward compatibility, upgrade an older version of suitecrm and add more joomla sites

at the admin panel, check is multiple validation type correct for us and do we need the validation here?
test somehow the marking as undeleted was success?
at updating portal, check which joomla url need for us to send because since it's multiple it does not make sense

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Final checklist

  • My code follows the code style of this project found here.
  • My change requires a change to the documentation.
  • I have read the How to Contribute guidelines.

gymad added 8 commits March 7, 2018 10:31
pick commits from old repo:

handle some user error

e17ffe0d

resolve some console error on SuiteP login

f5ed4899

09 May, 2017 8 commits

Merge branch 'staging' into 'develop-430/432-multi_aop' ...

89c9fe34

JAccount validation

af3bd372

error handling at JAccount save

a58cd219

handle array msgs

d91fac51

dbg refact

b0b02a9c

typo func name fix

694bbe3d

typo fix

db857ff9

add debug info for testing on dev

bad8fc93

08 May, 2017 5 commits

remove dbg and remove a php notice

c391e062

more informative begub and errorhandling for dev srv

46d5b4b9

signed exeption if no trace info

bc110409

handle some error and infos

098310ec

typo fix

4f6b489d

11 Apr, 2017 4 commits

upgrade portal url in config

7f3b6eca

creator portal url

77cf5f30

Merge branch 'develop' into develop-430/432-multi_aop

a0683125

gitignore autogenerated style.css.map

9435c1fd

10 Apr, 2017 1 commit

use only one related jaccount per contact/portal

7e3e7f79

07 Apr, 2017 12 commits

a style fix

8ffff73d

fix submenu funcs

1a4284e6

scss color link

f2c5f81e

a licens header added

8621a658

fix admin aop urls inputs

73896b0c

delete non used files

02feca9e

a typo

3ffc7960

aop xml check

5d5c736e

backward compatible

cf9c34e4

keep synch contact data for portal

d3d5ece0

refact

32bac9c7

create/enable/disable only at these portal url(s) where it's possible

18d72beb

05 Apr, 2017 1 commit

add multiple portal (code refact needed)

55798ad3

04 Apr, 2017 1 commit

add dropdowns

4b283ceb

03 Apr, 2017 1 commit

multiple portal url in AOP

f9babe7a
pick up commits from old repo:

class in separated files

fabf9c8b

check for email sent and fix a php notice

edb91aeb

fix email sending to created aop user

e7568f04

retrieve rest requested jaccount

3f804dcf

fix portal user creation email

7ae04322

15 May, 2017 2 commits

handle portal version error

01b891ea

refacts and fixes
@gymad gymad changed the title Develop multi aop WIP: Develop multi aop Mar 7, 2018
@gymad gymad force-pushed the develop_multi_aop branch from 234ba9d to f13fc84 Compare March 7, 2018 14:28
@gymad gymad changed the title WIP: Develop multi aop Develop multi aop Mar 7, 2018
@gymad gymad changed the title Develop multi aop Multi AOP Mar 7, 2018
.travis.yml Outdated
@@ -25,7 +25,7 @@ before_script:
script:
- \[ -f "config_si.php" \] || cp tests/travis_config_si.php config_si.php
- php tests/testinstall.php
- ./vendor/bin/codecept run -f unit --steps -vvv --debug
- ./vendor/bin/codecept run unit --steps -vvv
Copy link
Contributor

Choose a reason for hiding this comment

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

we need to ensure that codeception returns 1 as soon as it finds an issue in the tests. Please revert the option -f.

@@ -355,10 +363,10 @@ function quick_edit_case_updates($case)
$html = <<< EOD
<form id='case_updates' enctype="multipart/form-data">

<div><label for="update_text">{$mod_strings['LBL_UPDATE_TEXT']}</label></div>
<div><label for="update_text"></label></div>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we removing the translation in the label? Should we remove the translation here? What effect does removing this label have?

@@ -130,25 +136,39 @@ function aop_parse_template($string, $bean_arr)

foreach ($bean_arr as $bean_name => $bean_id) {

$focus = BeanFactory::getBean($bean_name, $bean_id);
if($focus = BeanFactory::getBean($bean_name, $bean_id)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This creates a PHP Notice. Consider moving the $focus to the line above, and then checking if (empty($focus)).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it doesn't, this line set the $focus.

* @return boolean
*/
function url_exists($url) {
if (!$fp = curl_init($url)) return false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Our coding standards states we must use {} for if statements. However perhaps it would be better to do something like this:

return  (curl_init($url) !== false);

curl_init returns false when an error occurs

@@ -72,11 +72,12 @@
$('#enable_aop').change();
$('#enable_portal').change(function (){
if($('#enable_portal').is(":checked") && $('#enable_aop').is(":checked")){
addToValidate('ConfigureSettings','joomla_url','text',true,"{/literal}{$MOD.LBL_AOP_JOOMLA_URL}{literal}");
$('#joomla_url_row').show();
// TODO: check is multiple validation type correct for us and do we need the validation here?
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove TODO.

'len' => 36,
'size' => '20',
),
'email1' =>
Copy link
Contributor

Choose a reason for hiding this comment

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

please define the email1 in the JAccount class.

'type' => 'varchar',
'len' => '255',
'importable' => 'false',
'studio' => 'true',
Copy link
Contributor

Choose a reason for hiding this comment

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

please define the joomla_account_id in the JAccount class.

'importable' => 'false',
'studio' => 'true',
),
'portal_account_disabled' =>
Copy link
Contributor

Choose a reason for hiding this comment

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

please define the portal_account_disabled in the JAccount class

'importable' => 'false',
'studio' => 'false',
),
'joomla_account_access' =>
Copy link
Contributor

Choose a reason for hiding this comment

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

please define the joomla_account_access in the JAccount class

'importable' => 'false',
'studio' => 'false',
),
'portal_user_type' =>
Copy link
Contributor

Choose a reason for hiding this comment

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

please define the joomla_account_access in the JAccount class

@horus68
Copy link
Contributor

horus68 commented Mar 7, 2018

Do @salesagility also plan to make some upgrade in the joomla plugin?
Its Github its almost dead since 2016, no PR merged (even the simplest ones), no issues solved.
See: https://github.com/salesagility/SuiteCRM-Portal-Joomla

Note: the ZIP download its also not available from your site!

@daniel-samson
Copy link
Contributor

@horus68 We will be updating the AOP plugin for joomla. So that it can support this feature and the new OAuth 2 server.

'LNK_NEW_RECORD' => 'Create JAccount',
'LNK_LIST' => 'View JAccount',
'LNK_IMPORT_JACCOUNT' => 'Import JAccount',
'LBL_SEARCH_FORM_TITLE' => ' JAccount',
Copy link
Contributor

@horus68 horus68 Mar 8, 2018

Choose a reason for hiding this comment

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

Is it intended to start with a space in ' JAccount',?

@samus-aran
Copy link
Contributor

@gymad Perhaps link this up with the appropriate Joomla plugin PR for people to test thoroughly.

@Dillon-Brown
Copy link
Contributor

@horus68 The zip can be found here: https://suitecrm.com/resources/suitecrm-joomla-portal-plug-in/

@horus68
Copy link
Contributor

horus68 commented Mar 8, 2018

@Dillon-Brown great to have this back again, even if just 2.0.1 version (nov 2016).
Hope the developments are made considering the 3beta branch, the 3.0.0 beta version (XML dated from October 2015), as it includes the feature for Advanced Open Knowledge Base

@gymad
Copy link
Contributor Author

gymad commented Mar 9, 2018

@samus-aran the link to the Joomla portal PR: salesagility/SuiteCRM-Portal-Joomla#25

@gymad gymad closed this Apr 26, 2018
@gymad gymad reopened this Apr 26, 2018
@Mac-Rae
Copy link
Contributor

Mac-Rae commented Aug 5, 2019

Please resolve the conflicts on this pull request bringing it up to date.

@Mac-Rae Mac-Rae added the Status: Requires Updates Issues & PRs which requires input or update from the author label Aug 5, 2019
@horus68
Copy link
Contributor

horus68 commented Oct 31, 2019

@gymad any update to solve conflicts here?

@chris001
Copy link
Contributor

No plan to resolve conflicts, update this PR or merge this multi joomla portal feature?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Requires Updates Issues & PRs which requires input or update from the author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants