Skip to content

[com_weblinks] Make ACL core.edit.own work (PR for 11466) #255

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

Merged
merged 6 commits into from
Aug 15, 2016
Merged

[com_weblinks] Make ACL core.edit.own work (PR for 11466) #255

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2b3ec8f
correct core.edit.own
andrepereiradasilva Aug 7, 2016
50f6900
fix conflicts 1
andrepereiradasilva Aug 7, 2016
ed336b0
Merge remote-tracking branch 'refs/remotes/joomla-extensions/master' …
andrepereiradasilva Aug 7, 2016
7001279
fix conflicts final
andrepereiradasilva Aug 7, 2016
0af9354
correct variable
andrepereiradasilva Aug 7, 2016
cfd842f
missing comma
andrepereiradasilva Aug 7, 2016
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
24 changes: 16 additions & 8 deletions src/administrator/components/com_weblinks/controllers/weblink.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,21 +60,29 @@ protected function allowAdd($data = array())
protected function allowEdit($data = array(), $key = 'id')
{
$recordId = (int) isset($data[$key]) ? $data[$key] : 0;
$categoryId = 0;

if ($recordId)
// Since there is no asset tracking, fallback to the component permissions.
if (!$recordId)
{
$categoryId = (int) $this->getModel()->getItem($recordId)->catid;
return parent::allowEdit($data, $key);
}

if ($categoryId)
// Get the item.
$item = $this->getModel()->getItem($recordId);

// Since there is no item, return false.
if (empty($item))
{
// The category has been set. Check the category permissions.
return JFactory::getUser()->authorise('core.edit', $this->option . '.category.' . $categoryId);
return false;
}

// Since there is no asset tracking, revert to the component permissions.
return parent::allowEdit($data, $key);
$user = JFactory::getUser();

// Check if can edit own core.edit.own.
$canEditOwn = $user->authorise('core.edit.own', $this->option . '.category.' . (int) $item->catid) && $item->created_by == $user->id;

// Check the category core.edit permissions.
return $canEditOwn || $user->authorise('core.edit', $this->option . '.category.' . (int) $item->catid);
Copy link
Member

@yvesh yvesh Aug 7, 2016

Choose a reason for hiding this comment

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

Can you save that in a variable before? That looks really not nice ;-)

Copy link
Contributor Author

@andrepereiradasilva andrepereiradasilva Aug 7, 2016

Choose a reason for hiding this comment

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

what variable? you want a $canEdit ? is it really needed .... ?

Copy link
Member

@yvesh yvesh Aug 7, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Just my opinion :) That return statement is not good readable imo.

Btw. Thank you for your great work!

Copy link
Contributor Author

@andrepereiradasilva andrepereiradasilva Aug 7, 2016

Choose a reason for hiding this comment

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

the idea is to only check authorize to core.edit if user cannot edit own.

Copy link
Member

@yvesh yvesh Aug 8, 2016

Choose a reason for hiding this comment

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

That's fine, but you could also save that to a variable ;-)

$canEdit = $canEditOwn || $user->..

return $canEdit;

But as i said that's just my opinion :-)

Copy link
Contributor

@mbabker mbabker Aug 8, 2016

Choose a reason for hiding this comment

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

Personally I'd say don't change it. Unless it's going to improve readability or going to be reused I try to not create variables needlessly. This is a case where it's doing neither IMO. Of course, coding styles and opinions have a lot of things in common 😉

Copy link
Contributor Author

@andrepereiradasilva andrepereiradasilva Aug 8, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

i'm like @mbabker on this. if we don't use a variable i don't like to create it.
but yes is more a matter of coding style.

}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/administrator/components/com_weblinks/models/weblinks.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ protected function getListQuery()
$query->select(
$this->getState(
'list.select',
'a.id, a.title, a.alias, a.checked_out, a.checked_out_time, a.catid,' .
'a.id, a.title, a.alias, a.checked_out, a.checked_out_time, a.catid, a.created_by, ' .
'a.hits, a.state, a.access, a.ordering, a.language, a.publish_up, a.publish_down'
)
);
Expand Down
5 changes: 3 additions & 2 deletions src/administrator/components/com_weblinks/views/weblinks/tmpl/default.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@
<?php $item->cat_link = JRoute::_('index.php?option=com_categories&extension=com_weblinks&task=edit&type=other&cid[]=' . $item->catid); ?>
<?php $canCreate = $user->authorise('core.create', 'com_weblinks.category.' . $item->catid); ?>
<?php $canEdit = $user->authorise('core.edit', 'com_weblinks.category.' . $item->catid); ?>
<?php $canCheckin = $user->authorise('core.manage', 'com_checkin') || $item->checked_out == $user->get('id') || $item->checked_out == 0; ?>
<?php $canCheckin = $user->authorise('core.manage', 'com_checkin') || $item->checked_out == $user->id || $item->checked_out == 0; ?>
<?php $canEditOwn = $user->authorise('core.edit.own', 'com_weblinks.category.' . $item->catid) && $item->created_by == $user->id; ?>
<?php $canChange = $user->authorise('core.edit.state', 'com_weblinks.category.' . $item->catid) && $canCheckin; ?>
<tr class="row<?php echo $i % 2; ?>" sortable-group-id="<?php echo $item->catid; ?>">
<td class="order nowrap center hidden-phone">
Expand Down Expand Up @@ -121,7 +122,7 @@
<?php if ($item->checked_out) : ?>
<?php echo JHtml::_('jgrid.checkedout', $i, $item->editor, $item->checked_out_time, 'weblinks.', $canCheckin); ?>
<?php endif; ?>
<?php if ($canEdit) : ?>
<?php if ($canEdit || $canEditOwn) : ?>
<a href="<?php echo JRoute::_('index.php?option=com_weblinks&task=weblink.edit&id=' . (int) $item->id); ?>">
<?php echo $this->escape($item->title); ?></a>
<?php else : ?>
Expand Down
2 changes: 1 addition & 1 deletion src/administrator/components/com_weblinks/views/weblinks/view.html.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ protected function addToolbar()
JToolbarHelper::addNew('weblink.add');
}

if ($canDo->get('core.edit'))
if ($canDo->get('core.edit') || $canDo->get('core.edit.own'))
{
JToolbarHelper::editList('weblink.edit');
}
Expand Down