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

Feature: Added document visits to page details #519

Merged
merged 15 commits into from
Apr 24, 2020

Conversation

Maaiins
Copy link
Contributor

@Maaiins Maaiins commented Apr 8, 2020

See: #335

@Maaiins Maaiins added enhancement New feature or enhancement Core Development Changes to framework or base module. labels Apr 8, 2020
@Maaiins Maaiins requested a review from padams April 8, 2020 17:18
@Maaiins Maaiins linked an issue Apr 8, 2020 that may be closed by this pull request
modules/base/module.php Outdated Show resolved Hide resolved
@Maaiins Maaiins requested a review from padams April 9, 2020 15:01
modules/base/module.php Outdated Show resolved Hide resolved
@padams
Copy link
Collaborator

padams commented Apr 10, 2020

Going back to the original request:

Allow seeing who visited the page and when

These two pieces of info could be gotten solely from owa_requests no? It has the session_id, document_id (for joining) and the timestamp...

@Maaiins
Copy link
Contributor Author

Maaiins commented Apr 11, 2020

Going back to the original request:

Allow seeing who visited the page and when

These two pieces of info could be gotten solely from owa_requests no? It has the session_id, document_id (for joining) and the timestamp...

Thats what i done first i copied the code from getLatestVisits and modified the joins accordingly

@padams
Copy link
Collaborator

padams commented Apr 11, 2020

getLatestVisits uses owa_session no? I can take a closer look in the morning.

@Maaiins
Copy link
Contributor Author

Maaiins commented Apr 11, 2020

It uses owa_session ;-)
image

Maybe there is some miscommunication. I decided to implement the feature as table resultset. At the moment I'm reusing the dashboard component to display the visitors. So you don´t have to take a look ;-)

@Maaiins
Copy link
Contributor Author

Maaiins commented Apr 12, 2020

image

Looks like these now

@padams
Copy link
Collaborator

padams commented Apr 18, 2020

Looks like my REST work blew up with PR. sorry! i should have merged this first.
can you fix the conflicts?

# Conflicts:
#	modules/base/module.php
#	modules/base/reportDashboard.php
#	modules/base/reportVisitor.php
#	modules/base/reportVisitors.php
#	modules/base/templates/report_document.tpl
@Maaiins
Copy link
Contributor Author

Maaiins commented Apr 18, 2020

Don´t worry not a big thing

@@ -79,13 +79,13 @@
prshre.load(prurl);

var vrurl = '<?php echo $this->makeApiLink(['do' => 'getResultSet',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Something is not right here. are you sure you have a full update from master? This could be the reason for the bug in #556

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was my addition, corrected it, but that's not the right place. I made a complete merge

Copy link
Collaborator

Choose a reason for hiding this comment

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

@padams
Copy link
Collaborator

padams commented Apr 21, 2020

Ok i meant to comment about your merge conflict in this window ;)

# Conflicts:
#	modules/base/module.php
@padams
Copy link
Collaborator

padams commented Apr 24, 2020

Looks like you just need to update the schema version.

@Maaiins
Copy link
Contributor Author

Maaiins commented Apr 24, 2020

Sorry was a little bit busy the last days

@Maaiins
Copy link
Contributor Author

Maaiins commented Apr 24, 2020

I just fixed it ;-)

@padams
Copy link
Collaborator

padams commented Apr 24, 2020

no worries!!

<span class="inline_h3"><a href="<?php echo $this->makeLink(array('do' => 'base.overlayLauncher', 'document_id' =>$document->get('id'), 'overlay_params' => base64_encode($this->makeParamString(array('action' => 'loadHeatmap', 'api_url' => owa_coreAPI::getSetting('base', 'rest_api_url'), 'apiKey' => $this->getApiKey(), 'document_id' => $document->get('id') ), true, 'json'))));?>" target="_blank">Heatmap Overlay</a></span> (Firefox 3.5+ required)
</P>
<p>
<span class="inline_h3"><a href="<?php echo $this->makeLink(array('do' => 'base.overlayLauncher', 'document_id' =>$document->get('id'), 'overlay_params' => base64_encode($this->makeParamString(array('action' => 'loadHeatmap', 'api_url' => owa_coreAPI::getSetting('base', 'api_url'), 'document_id' => $document->get('id') ), true, 'json'))));?>" target="_blank">Heatmap Overlay</a></span> (Firefox 3.5+ required)
Copy link
Collaborator

Choose a reason for hiding this comment

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

looks like you are reverting to the old style api_url here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

uh yeah wrong merged...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please check again ;-)

Copy link
Collaborator

@padams padams left a comment

Choose a reason for hiding this comment

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

looks good now. merging.

@padams padams merged commit 7507abf into master Apr 24, 2020
@Maaiins Maaiins deleted the feature_visits_page_detail branch April 24, 2020 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core Development Changes to framework or base module. enhancement New feature or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: Show views on the 'Page Detail' page
2 participants