Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#1261 closed New feature (fixed)

add: SitesManager getIdFromSite()

Reported by: oooa Owned by: vipsoft
Priority: normal Milestone: Piwik 0.6
Component: Core Keywords:
Cc: Sensitive: no

Description

For plugins/SitesManager/API.php, companion function to getSiteFromId()

This should search the database for a URL and return the ID of that site. This is extremely useful in keeping two databases synced (WPMU and Piwik in my case)

Matthieu says to remove the viewAccess check, but I'm only following convention from getSiteFromId(). Feel free to remove.

Sorry I don't have time to submit a proper patch.

	/**
	 * Returns the website information : id
	 * 
	 * @exception if the site website doesn't exist or the user doesn't have access to it
	 * @return string
	 */
	public function getIdFromSite( $site_url )
	{
		$site_url = $this->removeTrailingSlash($site_url);

		$id = Zend_Registry::get('db')->fetchRow(
					"SELECT Idsite FROM ".Piwik::prefixTable("site")." WHERE main_url = ?".
					"UNION SELECT Idsite FROM ".Piwik::prefixTable("site_url")." WHERE url = ?", array($site_url, $site_url));

		$id = isset($id['Idsite'])?$id['Idsite']:false;

		if ($id) { Piwik::checkUserHasViewAccess( $id ); }

		return $id;
	}

Change History (11)

comment:1 Changed 4 years ago by vipsoft (robocoder)

  • Milestone set to 1 - Piwik 0.6
  • Summary changed from New Feature to add: SitesManager getIdFromSite()

vote: stick with convention and keep the view access check.

comment:2 Changed 4 years ago by oooa

In light of being told that URLs may have multiple IDs, this should be changed to return an array:

$id = isset($id['Idsite'])?$id['Idsite']:$id;

Assuming Zend_Registry::get returns multiple rows?

Also, this line:

if ($id) { Piwik::checkUserHasViewAccess( $id ); }

would need to iterate over the IDs array on multiple matches and make sure each had access.

comment:3 Changed 4 years ago by matt (mattab)

I didn't say to remove the access check (of course!). My point was that this code is invalid because it will error out if one other websites (that you don't have access to) has the URL you are requesting. Only websites that you have permissions to view should be returned.

oooa, the Piwik::checkUserHasViewAccess function also accepts an array of IDs

did you test this code? I see it reads "Idsite" when the field name is idsite..

comment:4 Changed 4 years ago by vipsoft (robocoder)

Instead of checkUserHasViewAccess ... can you use the array of ids returned from getSitesIdWithAtLeastViewAccess() in the WHERE clause? i.e., idsite IN (?)

comment:5 Changed 4 years ago by matt (mattab)

probably, even though we want to avoid this as it doesn't scale (in the use case you have 2000 websites)

comment:6 Changed 4 years ago by vipsoft (robocoder)

The alternative is a subquery on the access table,

WHERE idsite IN (SELECT idsite FROM access WHERE user = ?)

comment:7 Changed 4 years ago by oooa

Idsite is capitalized because the select, I guess. Code does work as-is!

comment:8 Changed 4 years ago by vipsoft (robocoder)

AFAICS, comment:3 (comment:6 ?) and unit tests are still todo items.

comment:9 Changed 4 years ago by vipsoft (robocoder)

  • Owner set to vipsoft

comment:10 Changed 4 years ago by vipsoft (robocoder)

  • Resolution set to fixed
  • Status changed from new to closed

(In [2061]) fixes #1261 - SitesManager: add getSitesIdFromSiteUrl( $url ) to API; add unit test

comment:11 Changed 4 years ago by vipsoft (robocoder)

(In [2062]) refs #1261 - add unit tests for more complete test coverage

Note: See TracTickets for help on using tickets.