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

Add-on Update File - return values from parent installer class #715

Open
wants to merge 1 commit into
base: 7.dev
Choose a base branch
from

Conversation

RK311y
Copy link
Contributor

@RK311y RK311y commented Jul 29, 2023

Overview

In previous versions of the documentation, the value returned from install()/uninstall() is determined by the invocation of the parent class methods. See here.

Supporting Argument

Lets examine the routine which a add-on may encounter during installation (install()). All add-ons (which poses modules) must update the exp_modules database table. This process is taken care of by calling parent::install(). Then lets suppose that the add on needs to create a database table.

Steps

  • update exp_modules table
  • create add-on dependent database forge operations

If the order of these operation takes place such that the module is added and then the database tables are created, an error may occur in which the add-on is not successfully installed.

Current order of operations (see Sample Code below)

  1. update exp_modules table
  2. create add-on dependent database forge operations

Lets say for example that there is an issue when creating the database tables. The problem here is that the modules database table (exp_modules) has been updated to suggest that the add-on is in fact installed correctly. But during installation, the error occurred in which necessary database forge operations did not successfully complete.

Suggested order of operations

  1. create add-on dependent database forge operations
  2. update exp_modules table

This would allow the add-on to independently validate whether or not the installation process completed successfully before the parent operations took place.

Code Samples

Current Documentation

    public function install()
    {
        parent::install();

        // create a database table
        // notify mission control
        // add publish tabs

        return true;
    }

Old Documentaion

    public function install()
    {
        
        // create a database table
        // notify mission control
        // add publish tabs

        return parent::install();;
    }

Resolves #NN.

Nature of This Change

  • 🐛 Fixes a bug
  • 🚀 Implements a new feature
  • 🛁 Rewrites existing documentation
  • 💅 Fixes coding style
  • 🔥 Removes unused files / code

@intoeetive
Copy link
Contributor

I'd say it's up to developer if they want to handle in differently, but I would recomment to run parent::install first and then custom install routines. Might be subject for discussion

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.

2 participants