Skip to content

Correction of errors #1

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

Open
wants to merge 1 commit into
base: 7.x-1.x-donquixote-local
Choose a base branch
from
Open

Correction of errors #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion nestedbox_core.entity.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ function nestedbox_core_entity_info() {
// but without the ID suffix. (In fact, you can set
// entity_operations_entity_uri() as your URI callback, which will use the
// value here).
'path' => 'admin/content/nestedbox',
'path' => 'entity_operations_entity_uri',
Copy link
Owner

@donquixote donquixote Aug 13, 2016

Choose a reason for hiding this comment

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

I'm afraid this is not how it works. This key is meant to contain a path, not a function name.

Copy link
Owner

@donquixote donquixote Aug 13, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

You can look at EntityOperationsDefaultUIController::__construct() to see how the 'path' is used.

),
);

Expand All @@ -77,6 +77,7 @@ function nestedbox_core_entity_info() {
'label' => 'label',
),
'access callback' => 'nestedbox_type_access',
'static cache' => TRUE,
Copy link
Owner

@donquixote donquixote Aug 14, 2016

Choose a reason for hiding this comment

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

According to the doc comment on hook_entity_info(), this is TRUE by default. So no need to put it here.. right?

Copy link
Owner

@donquixote donquixote Aug 14, 2016

Choose a reason for hiding this comment

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

 *   - static cache: (used by DrupalDefaultEntityController) FALSE to disable
 *     static caching of entities during a page request. Defaults to TRUE.

Copy link
Contributor Author

@pifagor87 pifagor87 Aug 15, 2016

Choose a reason for hiding this comment

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

Yes

'module' => 'nestedbox_core',
// Enable the entity API's admin UI.
'admin ui' => array(
Expand Down
34 changes: 34 additions & 0 deletions nestedbox_type.admin.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,3 +78,37 @@ function nestedbox_type_form_submit_delete(&$form, &$form_state) {
$nestedbox_type = $form_state['nestedbox_type'];
$form_state['redirect'] = 'admin/structure/nestedbox-types/manage/' . $nestedbox_type->type . '/delete';
}

/**
* Nestedbox type delete form.
*/
function nestedbox_type_form_delete_confirm($form, &$form_state, $task_type) {
Copy link
Owner

@donquixote donquixote Aug 13, 2016

Choose a reason for hiding this comment

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

Wow, I notice this function was registered in hook_menu() all the time, but the function did not exist. So, good move to add this function!

if (!empty($task_type)) {
Copy link
Owner

@donquixote donquixote Aug 13, 2016

Choose a reason for hiding this comment

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

I think the wildcard loader nestedbox_type_load() will make sure this is never empty.

Copy link
Owner

@donquixote donquixote Aug 13, 2016

Choose a reason for hiding this comment

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

Hmm, it does not, currently, because the wildcard loader will return NULL instead of FALSE. We should change this.

Copy link
Owner

@donquixote donquixote Aug 14, 2016

Choose a reason for hiding this comment

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

And why is it "task_type", instead of "nestedbox_type" ?

Copy link
Contributor Author

@pifagor87 pifagor87 Aug 15, 2016

Choose a reason for hiding this comment

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

Erratum

$form_state['task_type'] = $task_type;
$form['task_type_id'] = array(
'#type' => 'value',
'#value' => entity_id('nestedbox_type', $task_type)
);
return confirm_form($form,
t('Are you sure you want to delete task type %title?', array('%title' => entity_label('nestedbox_type', $task_type))),
'task/' . entity_id('nestedbox_type', $task_type),
Copy link
Owner

@donquixote donquixote Aug 13, 2016

Choose a reason for hiding this comment

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

What is the purpose of entity_id() here?

Copy link
Owner

@donquixote donquixote Aug 14, 2016

Choose a reason for hiding this comment

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

oh i see

t('This action cannot be undone.'),
t('Delete'),
t('Cancel')
);
}
else {
$form_state['redirect'] = 'admin/structure/nestedbox-types';
}
}

/**
* Nestedbox type delete form submit handler.
*/
function nestedbox_type_form_delete_confirm_submit($form, &$form_state) {
$nestedbox_type = $form_state['task_type'];
entity_delete('nestedbox_type', entity_id('nestedbox_type' ,$nestedbox_type));
watchdog('nestedbox_type', '@type: deleted %title.', array('@type' => $nestedbox_type->type, '%title' => $nestedbox_type->label));
drupal_set_message(t('%title has been deleted.', array('%title' => $nestedbox_type->label)));
$form_state['redirect'] = 'admin/structure/nestedbox-types';
}