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.

Sign up for GitHub

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

Jump to bottom

Move shared module directory code to a shared file and support autoloading module interfaces #299

Merged
merged 26 commits into from
Dec 19, 2017
Merged

Move shared module directory code to a shared file and support autoloading module interfaces #299

merged 26 commits into from
Dec 19, 2017

Conversation

jsavell
Copy link
Contributor

@jsavell jsavell commented Sep 20, 2017

Resolves #297 - CORAL should behave the same, just with improved code reuse and maintainability

Resolves #298 - Module classes should be able to implement any interfaces defined under [module]/admin/interfaces without any manual require/include in that class file.

jeffnm and others added 11 commits February 27, 2017 14:47
@jeffnm
Additional Search Fields
2819a91 
Adding search fields for Publisher, Platform, and Provider.
@t4k
Fix PHP Notices on organizations landing page.
132eb12 
PHP message: PHP Notice: A session had already been started - ignoring session_start() in /var/www/coral/organizations/user.php on line 40
PHP message: PHP Notice: Undefined variable: letter in /var/www/coral/organizations/index.php on line 81
PHP message: PHP Notice: Undefined variable: letter in /var/www/coral/organizations/index.php on line 81
PHP message: PHP Notice: Undefined variable: letter in /var/www/coral/organizations/index.php on line 81
PHP message: PHP Notice: Undefined variable: letter in /var/www/coral/organizations/index.php on line 81
PHP message: PHP Notice: Undefined variable: letter in /var/www/coral/organizations/index.php on line 81
PHP message: PHP Notice: Undefined variable: letter in /var/www/coral/organizations/index.php on line 81
@t4k
Add a check in case user does not have first or last names.
d381b5e 
PHP Notice:  Undefined offset: 0 in /var/www/coral/organizations/admin/classes/common/DatabaseObject.php on line 87
@veggiematts
Issue 281: Fix button when adding a step in admin workflow edit
1c27089
fixing type in update script.
397709f
@jeffnm
Delete info.php
734ecd5 
We should not be including `phpinfo();` calls in the repository. It will cause unwary users to accidentally expose server configuration information.
@jsavell
refactor directory.php module files to share common code
38854da
@jsavell
move some language management to common_directory file
ddcf978
@jsavell
include common_directory from licensing module, include global LangCo…
3e3c814 
…des class
@jsavell
update autoloader to find module defined interfaces
0c076ed
@jsavell
fix license headers
10a6d00
@jsavell jsavell added enhancement This is an enhancement (not a bug) refactor labels Sep 20, 2017
@jsavell jsavell mentioned this pull request Sep 20, 2017
Merged
@jcuenod
Copy link
Contributor

jcuenod commented Sep 21, 2017 •
edited
Loading

Nice, this is going to be a long process...
What is the reason that you didn't move format_date() to the common file?

Also, this is where testing would be really beneficial for catching bugs introduced as "common" files become standard.

@jsavell
Copy link
Contributor Author

jsavell commented Sep 21, 2017

@jcuenod

I just didn't realize how many modules were using format_date()

@jcuenod
Copy link
Contributor

jcuenod commented Sep 24, 2017

Okay then also, regarding debug in common/common_directory.php:

  1. Is it being used anywhere (as far as I can tell there are a couple of places that it's commented out)
  2. Is it something that should be in the code base (I don't know what the feeling about that is).

@veggiematts
Copy link
Contributor

Tested on my instance, works for me.

veggiematts and others added 5 commits September 25, 2017 12:24
@veggiematts
Merge pull request coral-erm#295 from coral-erm/jeffnm-patch-5
3992765 
Delete info.php
@veggiematts
Merge pull request coral-erm#238 from caltechlibrary/issue77-php-noti…
69245b5 
…ces-organizations

Fix PHP Notices on organizations landing page.
@t4k
Issue coral-erm#77: Undefined index: auth/…/Utility.php Line 71
a5b0afc 
PHP Notice: Undefined index: HTTPS in ./auth/admin/classes/common/Utility.php on line 71
@veggiematts
Merge pull request coral-erm#309 from caltechlibrary/77-notice-auth-u…
df6bd68 
…tility-71

Issue coral-erm#77: Undefined index: auth/…/Utility.php Line 71
@veggiematts
Merge pull request coral-erm#290 from jcraitz/jcraitz-fix-SQL_typo-up…
1990daa 
…date_2.0.0

fixing typo in update script.
@t4k
Copy link
Contributor

t4k commented Oct 19, 2017

I question the filename common/common_directory.php. It seems redundant and I imagine that we will eventually have many files within the common directory. Are we going to prefix them all with common_?

@t4k
Copy link
Contributor

t4k commented Oct 19, 2017

Is there a reason the LangCodes.php file cannot be added to the common directory as well? I know the root index.php file uses it, but it would be nice if the root directory were cleaner and the common LangCodes.php file was actually located sensibly in common.

@t4k
Add checks to avoid PHP Notices in Resources code. (coral-erm#233)
4c5eef8 
* Fixes for PHP Notices on resources landing page.

PHP message: PHP Notice: Undefined index: orderBy in /var/www/coral/resources/admin/classes/domain/Resource.php on line 805
PHP message: PHP Notice: Undefined index: page in /var/www/coral/resources/admin/classes/domain/Resource.php on line 805
PHP message: PHP Notice: Undefined index: recordsPerPage in /var/www/coral/resources/admin/classes/domain/Resource.php on line 805
PHP message: PHP Notice: Undefined variable: currentPage in /var/www/coral/resources/index.php on line 32
PHP message: PHP Notice: Undefined index: startWith in /var/www/coral/resources/index.php on line 49
PHP message: PHP Notice: Undefined index: name in /var/www/coral/resources/index.php on line 69
PHP message: PHP Notice: Undefined index: resourceISBNOrISSN in /var/www/coral/resources/index.php on line 80
PHP message: PHP Notice: Undefined index: fund in /var/www/coral/resources/index.php on line 92
PHP message: PHP Notice: Undefined index: acquisitionTypeID in /var/www/coral/resources/index.php on line 125
PHP message: PHP Notice: Undefined index: acquisitionTypeID in /var/www/coral/resources/index.php on line 125
PHP message: PHP Notice: Undefined index: acquisitionTypeID in /var/www/coral/resources/index.php on line 125
PHP message: PHP Notice: Undefined index: acquisitionTypeID in /var/www/coral/resources/index.php on line 125
PHP message: PHP Notice: Undefined index: statusID in /var/www/coral/resources/index.php on line 150
PHP message: PHP Notice: Undefined index: statusID in /var/www/coral/resources/index.php on line 150
PHP message: PHP Notice: Undefined index: statusID in /var/www/coral/resources/index.php on line 150
PHP message: PHP Notice: Undefined index: creatorLoginID in /var/www/coral/resources/index.php on line 186
PHP message: PHP Notice: Undefined index: resourceFormatID in /var/www/coral/resources/index.php on line 211
PHP message: PHP Notice: Undefined index: resourceFormatID in /var/www/coral/resources/index.php on line 211
PHP message: PHP Notice: Undefined index: resourceFormatID in /var/www/coral/resources/index.php on line 211
PHP message: PHP Notice: Undefined index: resourceTypeID in /var/www/coral/resources/index.php on line 232
PHP message: PHP Notice: Undefined index: resourceTypeID in /var/www/coral/resources/index.php on line 243
PHP message: PHP Notice: Undefined index: generalSubjectID in /var/www/coral/resources/index.php on line 264
PHP message: PHP Notice: Undefined index: generalSubjectID in /var/www/coral/resources/index.php on line 275
PHP message: PHP Notice: Undefined index: detailedSubjectID in /var/www/coral/resources/index.php on line 295
PHP message: PHP Notice: Undefined index: noteTypeID in /var/www/coral/resources/index.php on line 356
PHP message: PHP Notice: Undefined index: noteTypeID in /var/www/coral/resources/index.php on line 366
PHP message: PHP Notice: Undefined index: noteTypeID in /var/www/coral/resources/index.php on line 366
PHP message: PHP Notice: Undefined index: noteTypeID in /var/www/coral/resources/index.php on line 366
PHP message: PHP Notice: Undefined index: noteTypeID in /var/www/coral/resources/index.php on line 366
PHP message: PHP Notice: Undefined index: noteTypeID in /var/www/coral/resources/index.php on line 366
PHP message: PHP Notice: Undefined index: noteTypeID in /var/www/coral/resources/index.php on line 366
PHP message: PHP Notice: Undefined index: resourceNote in /var/www/coral/resources/index.php on line 382
PHP message: PHP Notice: Undefined index: resourceNote in /var/www/coral/resources/index.php on line 384
PHP message: PHP Notice: Undefined index: createDateStart in /var/www/coral/resources/index.php on line 393
PHP message: PHP Notice: Undefined index: createDateEnd in /var/www/coral/resources/index.php on line 399
PHP message: PHP Notice: Undefined index: purchaseSiteID in /var/www/coral/resources/index.php on line 414
PHP message: PHP Notice: Undefined index: purchaseSiteID in /var/www/coral/resources/index.php on line 424
PHP message: PHP Notice: Undefined index: authorizedSiteID in /var/www/coral/resources/index.php on line 446
PHP message: PHP Notice: Undefined index: authorizedSiteID in /var/www/coral/resources/index.php on line 456
PHP message: PHP Notice: Undefined index: administeringSiteID in /var/www/coral/resources/index.php on line 478
PHP message: PHP Notice: Undefined index: administeringSiteID in /var/www/coral/resources/index.php on line 488
PHP message: PHP Notice: Undefined index: authenticationTypeID in /var/www/coral/resources/index.php on line 508
PHP message: PHP Notice: Undefined index: authenticationTypeID in /var/www/coral/resources/index.php on line 519
PHP message: PHP Notice: Undefined index: authenticationTypeID in /var/www/coral/resources/index.php on line 519
PHP message: PHP Notice: Undefined index: authenticationTypeID in /var/www/coral/resources/index.php on line 519
PHP message: PHP Notice: Undefined index: catalogingStatusID in /var/www/coral/resources/index.php on line 537
PHP message: PHP Notice: Undefined index: catalogingStatusID in /var/www/coral/resources/index.php on line 546
PHP message: PHP Notice: Undefined index: catalogingStatusID in /var/www/coral/resources/index.php on line 546
PHP message: PHP Notice: Undefined index: catalogingStatusID in /var/www/coral/resources/index.php on line 546
PHP message: PHP Notice: Undefined index: stepName in /var/www/coral/resources/index.php on line 568
PHP message: PHP Notice: Undefined index: stepName in /var/www/coral/resources/index.php on line 568
PHP message: PHP Notice: Undefined index: stepName in /var/www/coral/resources/index.php on line 568
PHP message: PHP Notice: Undefined index: stepName in /var/www/coral/resources/index.php on line 568

* Fix for PHP Notices on resources queue.

PHP message: PHP Notice:  Undefined variable: currentPage in /var/www/coral/resources/queue.php on line 24

* Fix for PHP Notices on resources admin.

PHP message: PHP Warning:  session_start(): Cannot send session cookie - headers already sent by (output started at /var/www/coral/resources/templates/header.php:183) in /var/www/coral/resources/admin/classes/common/CoralSession.php on line 25
PHP message: PHP Warning:  session_start(): Cannot send session cache limiter - headers already sent (output started at /var/www/coral/resources/templates/header.php:183) in /var/www/coral/resources/admin/classes/common/CoralSession.php on line 25

* Fix for PHP Notices on resources admin.

PHP message: PHP Notice:  Undefined variable: currentPage in /var/www/coral/resources/admin.php on line 24

* Fix for PHP Notices on resources resource.

PHP message: PHP Notice:  Undefined variable: currentPage in /var/www/coral/resources/resource.php on line 34
PHP message: PHP Notice:  Undefined index: ref in /var/www/coral/resources/resource.php on line 52
PHP message: PHP Notice:  Undefined index: workflowID in /var/www/coral/resources/admin/classes/domain/Resource.php on line 1976

* Fix PHP Notices on resources getUpdateProductForm.

PHP message: PHP Notice:  Undefined variable: resourceOrganization in /var/www/coral/resources/ajax_forms/getUpdateProductForm.php on line 306
PHP message: PHP Notice:  Undefined index: resourceID in /var/www/coral/resources/admin/classes/domain/Resource.php on line 1526

* Fix PHP Notices in resources getNewDowntimeForm.

PHP message: PHP Notice:  Undefined index: organizationID in /var/www/coral/resources/ajax_forms/getNewDowntimeForm.php on line 4
PHP message: PHP Notice:  Undefined index: issueID in /var/www/coral/resources/ajax_forms/getNewDowntimeForm.php on line 7
PHP message: PHP Notice:  Undefined offset: 0 in /var/www/coral/resources/admin/classes/domain/Downtime.php on line 67
PHP message: PHP Notice:  Undefined index: downtimeTypeID in /var/www/coral/resources/ajax_forms/getNewDowntimeForm.php on line 97
PHP message: PHP Notice:  Undefined index: shortName in /var/www/coral/resources/ajax_forms/getNewDowntimeForm.php on line 97

* Change isset() to be separate from is_array().
@jsavell
Copy link
Contributor Author

jsavell commented Oct 20, 2017

I don't have an opinion on the common_directory.php file name. I can change it do directory.php

I didn't investigate moving the root LangCodes.php file from where it was already located.

@t4k
Merge pull request coral-erm#286 from biblibre/Issue_281_Wrong_Button…
112a659 
…_In_Worfklow_Edit

Issue 281: Fix button when adding a step in admin workflow edit
@jsavell jsavell mentioned this pull request Nov 22, 2017
Closed
@veggiematts veggiematts self-requested a review December 14, 2017 15:18
@veggiematts
Copy link
Contributor

This patch breaks the API.
Please merge biblibre@143d1aa to your branch to fix the problem.
Otherwise, it seems all good.

veggiematts
veggiematts requested changes Dec 18, 2017
View reviewed changes
Copy link
Contributor

@veggiematts veggiematts left a comment •
edited
Loading

Choose a reason for hiding this comment

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

See previous comment

@jsavell
Copy link
Contributor Author

jsavell commented Dec 18, 2017

Thanks for catching and fixing that @veggiematts . The followup PR is merged in.

@jsavell
Copy link
Contributor Author

jsavell commented Dec 18, 2017

The new commits introduced by the followup PR merge all match hashes of commits already in the development branch, so I think it's just an artifact of how Github is presenting this older PR.

@veggiematts
Copy link
Contributor

As far as I'm concerned, we're good to go.

@veggiematts veggiematts merged commit dd0e2eb into coral-erm:development Dec 19, 2017
@PaulPoulain PaulPoulain added this to the Version 3.0.0 Beta milestone Mar 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@veggiematts veggiematts veggiematts approved these changes

Assignees
No one assigned
Labels
enhancement This is an enhancement (not a bug) refactor
Projects
None yet
Milestone
Version 3.0.0 Beta
Development

Successfully merging this pull request may close these issues.
6 participants
@jsavell @jcuenod @veggiematts @t4k @PaulPoulain @jeffnm