@remocrevo #56 (comment)

I believe the /usage/old/ directory should be removed, and therefore doesn't need to be updated. Can anyone confirm?

In the Resource module, the term "workflow" is used, while the tab in the resource is called "routing".
We must be consistent.
As the DB table is called "Workflow", I suggest to rename "routing" to "workflow" for the resource tab

In many areas of Coral, there is an "Edit xxxx" link AND an icon to edit. We don't need 2 areas to do the same thing, on the same screen. It can be confusing for users

During the install I got multiple times these messages:

• PHP Warning: mysqli_error() expects parameter 1 to be mysqli, boolean given in /srv/http/coral/licensing/install/CORALInstaller.php on line 57, referer: http://localhost/coral/licensing/install/install.php
• PHP Warning: mysqli_error() expects parameter 1 to be mysqli, boolean given in /srv/http/coral/usage/install/CORALInstaller.php on line 69, referer: http://localhost/coral/usage/install/install.php

Other than #58, it shows that when CORALInstaller is instantiated, it calls connect without any arguments, which leads to a connection failure but it doesn't seem to affect the install.

In that case, the credentials should come from the config file but on a fresh install, it's empty.

It's suspicious, because it should be handled instead of trying to connect and causing a "false failure" right?

On other modules I remember seeing somewhere a message about trying to log to mysql with NULL credentials and thought it was a security check. But it was actually this issue without #58 interfering.

In auth the function of the session timeout variable is not clear. The setting is used to set the user cookie expiration timeout. It should probably have a clearer name in the config file.

The naming standards section of the Technical Documentation should be linked in the Coding guidelines.

As naming standards are what one expects to find when looking for coding guidelines.

When an email is sent using templates, some variables are expanded, like ResourceTitle or ResourceID (see resources/admin/emails/)

The same goes for the resource url variable: ResourceRecordURL

However, this does not work when the message is processed by a cronjob.

Indeed, the function used to expand the URL variable (getCORALURL) relies on $_SERVER variables, which are not populated when in cron environment.

The produced URL looks like this: http://:/home/coral/www/resources/resource.php?resourceID= , which is wrong.

Some fix ideas:

• Store the CORAL URL in configuration.ini
• use curl or wget in the crontab, like this:
  /usr/bin/curl http://domain.tld/path/to/cron-script.php &> /dev/null

The suspicious pattern is HTML generated in a try block which a part will be skipped if an exception is thrown. Thus the catch displaying the error should be carefully designed to complete the HTML and display it's error message.

After checking all the catch of Coral here are some very suspicious places and they should be manually triggered to confirm that they are visible and not affect by the same bug as #64

• https://github.com/Coral-erm/Coral/blob/master/management/index.php#L95
• https://github.com/Coral-erm/Coral/blob/master/management/ajax_htmldata.php#L590
• https://github.com/Coral-erm/Coral/blob/master/licensing/index.php#L91
• https://github.com/Coral-erm/Coral/blob/master/licensing/ajax_htmldata.php#L495
• https://github.com/Coral-erm/Coral/blob/master/licensing/ajax_forms.php#L93
• https://github.com/Coral-erm/Coral/blob/master/organizations/summary.php#L623
• https://github.com/Coral-erm/Coral/blob/master/organizations/ajax_htmldata.php#L860

The new fields added to the enhanced cost history are not showing up in the print view. These fields need to be added to the print view. Also add notes fields to print view.

Currently, notes are not part of the export function in Resources. They also don't display in print format. We should add them as they are a very useful source of information.

the org and licensing modules use an older style for the edit windows where the layout is more stacked. the later resources module changed to a more broadly spaced layout for the edit windows. this issue is a request to update the org and licensing module edit windows to the layout used by the resources module.

• The one on / is still the old one
• It's missing in the Licensing and Auth modules (linked in HTML but not in ./images)

We need to apply the changes from the old standalone repos final 1.x releases so that we have the same base for the new upgrade processes.

https://github.com/ndlibersa/

There are unresolved merge markers (i.e.)

<<<<<<<HEAD
...
=======
...
>>>>>>> ...

in the .php and .sql files in the reports/install directory.

Restart a completed workflow: Users should be allowed to restart a new workflow by selecting a workflow from a pre-defined workflow list or restart the current completed workflow. The current CORAL functionality does not allow users to restart any already completed workflow.

Ability to archive previous workflows: Previously completed workflows contain valuable information, such as date when tasks are completed, and such information should be kept. Currently CORAL does not support such functionality.

Ability to add a step in the workflow for a particular resource: The ability to add a step offers users greater flexibility to determine a customized workflow for a specific resource. CORAL provides a globalized add a step function in the Admin section, which applies to all resources associated with this workflow, not on individual resource level.

Receive assignment email notification with some context about the assignment: Users have complained that the workflow assignment emails sent from CORAL do not contain much useful information. It would be helpful to embed a concise message in the email about the assignment rather than just “a resource is added to your queue” as is currently displayed.

Send email reminder to users who are assigned to a step: Task reminders are often seen in project management application or task management tools. This functionality will remind users to complete the steps by a due date. CORAL has alert function in place now, but only for subscription alerts.

Hello,
I found the data models, and looking for the mapped out the workflows of the various modules in practice?

Currently there doesn't seem to be detailed instructions for someone who knows nothing about Coral to deploy a working instance. Whether it's for contributing or using Coral.

What would be better?

• an INSTALL.md file
• a wiki page linked from the README

Technical Documentation and Install Guide for all module on http://coral-erm.org/documentation/
Links to https://github.com/ndlibersa/CORAL-Main/wiki/CORAL-Technical-Documentation

Should it link to https://github.com/Coral-erm/Coral/wiki/CORAL-Technical-Documentation ?

In the "new document" form, this prevents the Categories select to show the created consortiums.

Manually editing management/admin/configuration.ini allows to use the consortiums.

note: management/admin/configuration.ini contains also Usage and Resources settings and the install also doesn't expose them.

https://github.com/Coral-erm/Coral/blob/b63857f802d7707abfdaccee72528752b6cba5e1/resources/admin/classes/domain/Resource.php#L293

The query returns at most one result. But what happens (and should happen) when there is no license? (Is it possible?) Because the code not explicitly handling/commenting that case is suspicious.

The comment that was copypasted from other parts of the code where multiple results can happen also add to the suspicion.

Also the SQL query seems to unnecessarily use a nested query and non standard LIMIT, offset, count

After looking at the different DBService of each module, here are the current behaviors:

not migrated yet from mysqli to mysql and disconnect() is doing nothing:
https://github.com/Coral-erm/Coral/blob/master/usage/old/admin/classes/common/DBService.php#L55
https://github.com/Coral-erm/Coral/blob/master/management/admin/classes/common/DBService.php#L57

migrated to mysqli and disconnect() is doing the close():
https://github.com/Coral-erm/Coral/blob/master/usage/admin/classes/common/DBService.php#L52

migrated to mysqli and disconnect() is still doing nothing:
https://github.com/Coral-erm/Coral/blob/master/auth/admin/classes/common/DBService.php#L53
https://github.com/Coral-erm/Coral/blob/master/licensing/admin/classes/common/DBService.php#L54
https://github.com/Coral-erm/Coral/blob/master/resources/admin/classes/common/DBService.php#L56
https://github.com/Coral-erm/Coral/blob/master/organizations/admin/classes/common/DBService.php#L56

migrated to mysqli but no disconnect() nor dealloc() and it's the only DBService which doesn't extends Object:
https://github.com/Coral-erm/Coral/blob/master/reports/admin/classes/common/DBService.php

• Which one is correct? The git history doesn't go a far as when the close() where commented.
• Is the DBService of the Reports module correct? (no dealloc() and not extending Object)

It's more a discussion about what should be the priorities to keep the codebase maintainable and how can we improve it incrementally.

Duplication between modules

Whether it's in the database or in the code itself, having now everything in the same repo allows to share stuff.

For example it seems that generic classes like DBService or other abstractions are 99% the same between modules. So for example when finishing the conversion from mysql to mysqli in Management, it would have been useful to study the possibility of merging the DB related classes with another module that was already converted. Instead of doing the same work and contributing to make them diverge in meaningless ways.

Coding style

• tabs in most places, spaces in some and even mixes of them (see #55)
  • alignment are often broken (in different ways depending of the editor settings!)
  • difficulties to match related blocks (if/else, especially when there is nesting) which leads to comprehension errors
• trailing whitespaces
• windows end of lines in some files
• inconsistent spacing
  • while/if (condition) { vs while/if (condition){
  • } else { vs }else{

For reasons difficult to explain, among a non negligible share of developers, these can create a lot of frustration and distract from getting work done.

One strategy that I'm experimenting is before making non trivial changes to a file (and having to deal with these style issues) I do a quick pass to fix the most blatant issues once for all in a separate commit.

For now it seems to work well.

Eliminating notices and warnings

When using Coral each click usually generates dozens of them and spams the logs with undefined variables, indexes, offsets, etc. Making difficult to spot one's owns mistakes, hiding existing bugs, making investigations more difficult.
Example of the noise added when investigating an error and how it can worry the users (ops in the libraries) which end up disabling the error logging.

It seems that there are not too many different causes and therefore gradually adding simple checks would allow to shut these messages.

Anything that could be done better? Because these strategies are a bit of improvisation given the low experience with Coral and PHP that I have.

What are the other things that we could do to gradually improve the code and make Coral more fun to develop?

So users who don't take the time to read the instructions can't submit the form with the names of the CSV columns instead of the indexes
(no I haven't done that!)

During the install I got multiple times these messages:

• PHP Warning: mysqli_error() expects parameter 1 to be mysqli, boolean given in /srv/http/coral/licensing/install/CORALInstaller.php on line 57, referer: http://localhost/coral/licensing/install/install.php
• PHP Warning: mysqli_error() expects parameter 1 to be mysqli, boolean given in /srv/http/coral/usage/install/CORALInstaller.php on line 69, referer: http://localhost/coral/usage/install/install.php

The problem was in the mysql → mysqli conversion, because replacing mysql_connect by mysqli_connect is not enough. Here is an example of a correct one:
ndlibersa/resources@26ae17c#diff-2eb558a4ae0bd19dc896cd22fadbfa52L85

• TODO: search for other wrong mysqli instantiations.

Motivations

Separate business logic from presentation logic/views to:

• work more easily when editing the views only (easily see what the HTML code will be)
• reliably implement logic without risking to break the view(example: #73)
• reduce duplication and code necessary to generate the HTML
  • consistency across the whole app for the same components

We need a template engine because:

• doing it in pure PHP (no library) is hard (inheritance and reusable blocks) and require discipline to keep the logic out
• in the end we would build our own engine: see Plates and Foil for engines with a similar philosophy
• it's not possible to achieve the same level of readability with pure PHP. (one, maybe not representative example of a project where they have their own engine) see also Plates and Foil.

Caveats/Disclaimer

I only have ~50 hours of experience with PHP and Coral so the following initial work is expected to contain, errors, biases and irrelevances about

• the chosen criteria
• the listed alternatives
• the chosen characteristics, pros and cons of the alternatives

That being said, any template engine will allow to make huge progress toward the goals listed in the Motivations section.
So there is no choice that is inherently bad. (except never choosing)

And the work to switch from one engine to another should in any case be lower than the work to separate the views from the business logic (for which a template engine will help a lot).

So don't be afraid that we could take decisions that might have terrible future consequences for the development of Coral. That's a perk of the current situation, we will necessarily remove more technical debt than increasing it.

Choice criteria

• how far can it go to help us making Coral better
  • good, useful and comprehensive features → quality of result
• ease of implementation (cost)
  • bonus if long term Coral developers have experience with it
• long term sustainability: because it's better if there is no need to change it
  • will still be maintained for many years
  • bonus if long term Coral developers have experience and feel good with it because they will benefit the most from this choice

Alternatives

Twig

• comprehensive DSL
  • handle in a readable way all the view logic(conditions, loops)
  • share code between templates to reduce duplication (inheritance, reusable blocks)
  • syntax similar to other template languages like Django's and Jinja so learning the language can be even easier when already having experience with a template language.
• flexible: for example there is Twital, a plugin that changes the DSL to one closer to HTML which we might prefer compared to the default one.
• documentation said to be good
• future proof/will last many years as it's very popular and it's the template engine chosen by Symfony and Drupal 8
• easy to find help and resources online due to it's popularity
• seems to be more "polished" than Smarty which still carries some legacy
• seems to easier to integrate with bigger projects than Smarty
• I18n is straightforward as we can directly call to the I18n function in the template.

Smarty

• DSL at bit less comprehensive than Twig but not sure if anything important is missing

Plates

• no DSL → PHP with helper functions
  • heavier to write than a DSL, but less than pure PHP
  • less readable (more boilerplate code but less than pure PHP)
• inheritance
• reusable blocks
• needs discipline to keep logic and DB interactions out
• seem to lack some useful features of Twig²
• user base a bit small

Foil

• no DSL → PHP with helper functions
• needs discipline to keep logic and DB interactions out
• fixes some of the criticisms made to Plates²
• user base really small
  • future proof concerns
  • finding help will be difficult

Mustache

• logic-less templates: just send an array with the values to be replaced in the template
  • enforces separation of concerns as all logic must be out, no slippery slope that can lead to complex templates
  • simpler syntax to learn compared to classic engines but being forced to keep the logic out also adds it's own difficulties
  • still allows to achieve results like with conditions and loops
  • reusable blocks (partials)
  • inheritance
• possibly limited in some use cases but I can't figure out their relevancy³
• compared to classic engines like Twig, it seems that usually the logic-less nature of Mustache makes it harder to implement but on the long term, not being able to put logic in a template keeps them maintainable. ⁵
  • actually it could be easier to use when no template experience⁴
• I18n seems tricky but doable ⁶
• If we plan to someday send JSON to the client(browser) instead of HTML to let it do the rendering(or mutualize the work for making an API), then we could switch to Mustache.js at a low cost by reusing a great part of the templates.

Rain TPL

• Dead project
  • http://raintpl.com/
  • https://rainphp.github.io/raintpl
  • https://github.com/feulf/raintpl/graphs/contributors
• finding help is difficult
• Might be too simple and limited for a big project like Coral

Some of the references when searching the differences between the alternatives

[1] https://www.quora.com/What-are-the-pros-and-cons-of-Smarty-versus-Twig-php-web-templates

[2] http://gm.zoomlab.it/2015/template-engines-i-moved-from-love-to-meh-for-plates/

[3] Search for "as little logic as possible" in http://www.sitecrafting.com/blog/top-5-php-template-engines/

[4] Someone on the #php IRC:
I'd say using Mustache makes writing maintainable templates easier for developers with no template experience, IF they understand why Mustache is limited

[5] Someone on #php IRC:
Separation of concerns. also better IDE support (for refactoring, find usages etc) and using the tool better designed for expressing business logic: a programming language
(and yes, IMO display logic like "show the profile as yellow if the subscription is about to end" is business logic too)
and that's where discipline comes in it's getting harder and harder to decide what should be "in" and what should be "out"

[6] I18n with Mustache.php

• https://github.com/bobthecow/mustache.php/blob/master/test/fixtures/examples/i18n/I18n.php
• bobthecow/mustache.php#291 (comment)
• https://stackoverflow.com/questions/8695970/mustache-i18n-with-parameters

Conclusions

If I didn't missed something huge then the choice would be between Twig and Mustache.

To make the final choice I think if one or some of the Coral developers have experience with one of the two then it will make the difference.
It will weight more in the balance than the difference found between Twig and Mustache (especially with the Caveats that alone create an error margin where the difference between Twig and Mustache fits)

So this is where we need feedback from those of us that already worked with template engines.

I don't have a live install right now - I'm just building an installer. I've just merged in the visual changes by Bradley Droubay and I'm getting

Notice: Undefined index: en_GB in /srv/http/Coral/LangCodes.php on line 15

Looks like a simple fix - seems it's just missing from an array.

Currently, the import tool allows the user to map only a few fields from the Resource table to CSV columns.

My proposal is to increase the number of fields available in step 2 of the import tool, so the user can map any field of the Resource table.

For convenience, if a CSV column fit the MySQL database name, then the link will be automatically made and proposed by default.

We at Linköping University Library in Sweden have installed the latest upgrades to the CORAL modules and experienced some problems with the UTF-8 characters. We looked at the source code and have suggestions to solve these problems. The changes we propose works without side effects in our installation but there might of course be other ways to solve these problems.

1. Organization names does not display correctly in Resources and Licensing

Organization names (coming from the Organizations module) with Swedish characters are not displayed correctly in Resources and Licensing.

All tables in the organizations database are defined as UTF-8 as in the resources and licensing modules. But when comparing opening the databases (in the program DBService.php) we see that both resources and licensing are opened as charset utf-8 but that organizations isn't.

In resources/admin/classes/common/DBService.php
we find the line (line 49):
$this->db->set_charset('utf8');

In licensing/admin/classes/common/DBService.php
we find the line (line 51):
mysqli_set_charset($this->db, 'utf8');

In organization/admin/classes/common/DBService.php we don't find any of the above commands or similar.

By adding on of the above commands the Swedish characters displays correctly in the other two modules.

2a. Organizations names does not display correctly when editing in the Organizations form
2b. Alias short names does not display correctly when editing in the Resources product form

Looking at the code we found that htmlentities causes the problem. Searching on internet we found several references about htmlentities and utf-8 characters (for example https://boulderinformationservices.wordpress.com/2011/12/31/utf-8-not-working-look-for-htmlentities/ or http://stackoverflow.com/questions/5679715/htmlentities-destroys-utf-8-strings).

We tested with the following changes which does display all characters correctly.

organizations/ajax_forms.php
changed from
<td><input type='text' id='organizationName' name='organizationName' value = "<?php echo htmlentities($organization->name);
to
<td><input type='text' id='organizationName' name='organizationName' value = "<?php echo htmlentities($organization->name, ENT_QUOTES, 'UTF-8');

resources/ajax_forms/getUpdateProductForm.php:
changed from
<input type='text' value = '<?php echo htmlentities($resourceAlias['shortName'], ENT_QUOTES);
to
<input type='text' value = '<?php echo htmlentities($resourceAlias['shortName'], ENT_QUOTES, 'UTF-8');

Please let me know if clarifications or examples (screenshots) are needed

The upgrade changed the defined character se to utf-8 for all tables. But we also had to convert the content of the tables to utf-8 characters.

We are using CORAL in production since 2011 and are also a SirsiDynix library.

Regards
Thomas Trakell
Linköping University Library
Sweden

There are a lot of places (I found at least 37) where like is used with upper (find them with grep -ri "like upper"). For example: management/admin/classes/domain/Signature.php has:

WHERE upper(signerName) like upper('%" . $q . "%')

LIKE is case insensitive though and so upper is unnecessary. Technically it can be made case sensitive with binary comparisons - using the utf8_bin collation or ... LIKE BINARY ... but this is clearly an attempt to be case insensitive.

When opening the form:

When validating:

The "helpful link" area, on the right:

• takes a lot of space
• has duplicate informations

for example: http://coraldemo.library.tamu.edu/resources/resource.php?resourceID=75

• the "child records" is also displayed as Related products in the main area
• the Organization module: Ebrary is also displayed as "Organizations: "

@jsavell

Right now, our data classes are sprinkled with view and business logic and our controllers and views are merged into one entity. This greatly complicates modifying code or adding new features.

Are there already discussed ideas about how to start progressing toward this goal?

There are currently some inconsistencies leading to indentation issues between GitHub's code view and the different text editors out there.

We should choose one style and add it to the guidelines so we will gradually unify the indentation style as when new code will get written, it will fix the existing inconsistencies.

After few minutes jumping through the 70 000 lines of PHP code of Coral, I've found that > 90% of the indentation are tabs.

So tabs would be the simplest choice to avoid a mass conversion. In the case where tabs are chosen, the length should be chosen to preserve alignment (for example in a function with many params on multiple lines)

When setting up the project with PHP 7
And running going to /management/install/
I see in the logs Call to undefined function mysql_connect() in /srv/http/coral/management/install/CORALInstaller.php:35
Which is a function of the mysql PHP extension
Which has been removed in PHP 7, as summed up here

There was already work done in the admin and install modules.

Grepping in the code shows that the Management and Report module are the last places using mysql_connect() and mysql_query()

I'll try to migrate them using the existing work.

Steps to reproduce:

• go to /auth
• the language drop down should contain english and french
• mv auth/locale auth/locale.bak
• refresh the page
• the language drop down should only contain english
• and the HTML contains

    <div class='boxRight'>
        <p class="fontText">Change language:</p>
        <select name="lang" id="lang" class="dropDownLang">
            <br>Invalid translation route!<option value='en_US'>English</option>
         </select>
    </div>

• but it's not visible on the page

In the resources module a resource's alias is an element of the Product tab. In the organizations module an org's alias is given its own dedicated tab within the record. this issue is to request that we select an approach and use the same for each module.

Administering, authorized and purchase Sites only contain a description. This description can be long. It would be useful to have a code/short description added and used in some places

Coral is currently missing an API, in order to be able to gather informations or interact with coral from within third-part applications.

For a start, I would be working on proposing a new resource from an external form, but many other things could be achieved afterwards.

A REST API is what I consider to be the best, as it's simple and efficient.

REST API are rarely developped from scratch: libraries are often used, in order to focus on the API itself, and not the HTTP technical stuff behind it.

I've checked several PHP REST libraries, and Flight seems to be an ideal candidate, as it's simple, lightweight, and quick to use.
http://flightphp.com/

Before dealing with more complex authentication systems (like token), a simple IP-based authorization would be easy to implement and operate.

In a resource detail screen, we display the resourceID
Record ID: 227077

There's no reason for displaying this field, and no ID is not displayed in other areas of Coral (which is good)

When trying to install the latest version of Coral resources, I get this error:

Invalid default value for 'updateDate'

For statement: CREATE TABLE ResourceNote ( resourceNoteID int(11) NOT NULL auto_increment, resourceID int(11) default NULL, noteTypeID int(11) default NULL, tabName varchar(45) default NULL, updateDate timestamp NOT NULL default '0000-00-00 00:00:00', updateLoginID varchar(45) default NULL, noteText text, PRIMARY KEY (resourceNoteID) ) ENGINE=MyISAM AUTO_INCREMENT=1 DEFAULT CHARSET=utf8`

I am using PHP 5.6 and MySQL 5.7

For newer verisons of MySQL, strict mode has been changed and you cannot use '0000-00-00' as a default date without changing some settings in MySQL.

My suggestion instead would be to instead set the default value to be "NOW()", or to enable NULL values.

http://dev.mysql.com/doc/refman/5.7/en/sql-mode.html#sql-mode-changes
http://stackoverflow.com/questions/25349126/how-can-i-set-the-default-value-of-a-field-as-0000-00-00-000000

Happy to make a PR for this if it sounds like a reasonable solution, however I'm not entirely sure yet how that field is used or what makes most sense for existing installs, or how this would be handled in an upgrade.

All modules should provide the same notes features as the Resources module has. The notes code and database tables should be shared among all modules.

Begin with usage because it has few dependencies to the other modules.

In the org module, when you create/update an organization, you can select the role(s) it can have.

In the resources module when you create/update a resource and attach an organisation, once you've chosen the role, the auto-complete does not limit the results to orgs that can have the selected role

New fields added to enhanced cost history need to be added to the export feature. Note it appears dates for subscriptions are listed in export, but the dates are not actually exporting.

The license is currently displayed in the "acquisition" tab. It is more logical to put it as a specific tab on the left (with acces, cataloging, contacts...)

According to the answer on this page: http://stackoverflow.com/questions/766809/whats-the-difference-between-utf8-general-ci-and-utf8-unicode-ci/766996#766996

MySQL 5.5.3 and up use utf8mb4, rather than utf8, which is the same UTF-8 data format with the same performance but removes a MySQL-specific limitation where only characters up to xFFFD were supported.

I think we should switch our installation and upgrade scripts to use utf8mb4. Are there any objections? Concerns?

With the unified/merged repository we can (and should) merge the po files into just one. That will ease a lot the translation process

When importing a file encoded with windows-1252 (AKA latin1)
Which contains strings such as "générale"
On a system with an UTF-8 locale.
Then the result is "g?n?rale"

see SC members for this issue, that has been declared privately for security reasons

The tab menus, on the left, are not translatable