ISAICP-6507: Edit tooltip for JLA entry "Disclose source". #2511
Conversation
There was a problem hiding this comment.
Apart from the specific remarks, there are no test for the functionality, please, write a (preferably) behat test of type:
- As a user of the website
- I go to the JLA page
- And the response should contain "html with the tooltip inserted here"
The "And the response should contain" step asserts the returned HTML and you can check that the HTML that is needed for the tooltip is there.
| /** | ||
| * {@inheritdoc} | ||
| */ | ||
| public static function create(ContainerInterface $container) { |
There was a problem hiding this comment.
If there are no services needed for the service, simply do not implement the ContainerInjectionInterface and do not implement the ::create method.
| /** | ||
| * Replace current JLA list. | ||
| */ | ||
| public function updatejla() { |
There was a problem hiding this comment.
To be fair, it is a good idea to have a helper method like that. But we are duplicating work because we already have a command to import the fixtures, in \Joinup\TaskRunner\Commands\VirtuosoCommands::importFixtures.
IF we were to keep it, maybe we could make it configurable. Accept a parameter for the filepath and one parameter for the graph URI. Also, rename the method name to something more generic like replaceFixturesInGraph. Otherwise simply remove this and move the code to the update hook.
| /** | ||
| * Update the Licence legal type vocabulary. | ||
| */ | ||
| function joinup_sparql_post_update_0107301(array &$sandbox): void { |
There was a problem hiding this comment.
In Joinup, we are deploying using the drush deploy command (ref. https://www.drush.org/latest/deploycommand/).
This is adding the following sequence:
- Run hook_update_N functions.
- Run hook_post_update_NAME functions.
- Import configuration.
- Run hook_deploy_N functions.
Since post_updates run before configuration import, we generally prefer to run simple updates that use the API after we run the configuration import.
Please, move this function to the joinup_core.deploy.php. The numbering and sequence retain the mechanism of the 01_073_00 (without the underscores) that you wrote above. Also, the first update function, weather update, post update or deploy hook, starts with the 00, not the 01 from above.
There was a problem hiding this comment.
Please, note that, the drush update does not run the deploy hooks so it is not a Drupal Core official function. The drush deploy runs the full sequence.
No description provided.