Skip to content

Comments

9647 bugfix facets on files page#9649

Merged
kcondon merged 3 commits intoIQSS:developfrom
ErykKul:9647_bugfix_facets_on_files_page
Jun 16, 2023
Merged

9647 bugfix facets on files page#9649
kcondon merged 3 commits intoIQSS:developfrom
ErykKul:9647_bugfix_facets_on_files_page

Conversation

@ErykKul
Copy link
Collaborator

@ErykKul ErykKul commented Jun 16, 2023

What this PR does / why we need it:
It fixes the missing filters for files in dataset page bug.

Which issue(s) this PR closes:

Closes #9647

Suggestions on how to test this:
Open a dataset page for a dataset with more than 1 file, it should contain the facet filters (Filter by)

Does this PR introduce a user interface change? If mockups are available, please link/include them here:
No, this is only a bugfix.

Is there a release notes update needed for this change?:
No.

Additional documentation:
image

@ErykKul
Copy link
Collaborator Author

ErykKul commented Jun 16, 2023

@pdurbin @kcondon
This pull request fixes a bug introduced in my PR #9558, I have missed a side effect there and the facet were not set when opening a dataset page. The change is very limited: only 2 lines of code, the rest is comments. Sorry for the late notice.

Copy link
Member

@qqmyers qqmyers left a comment

Choose a reason for hiding this comment

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

makes sense. QA should be able to verify the fix works given the problem description.

@kcondon kcondon self-assigned this Jun 16, 2023
@landreev
Copy link
Contributor

Curiously, I just spent some time purposefully hiding these file facets from googlebot on our prod. servers. I was taken aback by how much unnecessary crawling they were responsible for (again).
In light of which, I'm not even sure if this was a bug, or a welcome feature.

@ErykKul
Copy link
Collaborator Author

ErykKul commented Jun 16, 2023

@landreev
I think that attribute rel="nofollow" should do the trick:
image
https://developers.google.com/search/docs/crawling-indexing/qualify-outbound-links

If it is not what is desired, let me know and I will revert it.

@kcondon
Copy link
Contributor

kcondon commented Jun 16, 2023

@ErykKul @qqmyers This works but a minor issue is that the filter doesn't appear right away when a draft is created from a published dataset -page needs to be refreshed. Is this because async not available at time of page init? I can merge as is since it is minor, just wanted to mention it.

@ErykKul
Copy link
Collaborator Author

ErykKul commented Jun 16, 2023

@kcondon
I think this is OK. Probably, at that time the index is not ready yet and the isIndexed is false for the draft version.
What do you think about the "nofollow" thing?

@landreev
Copy link
Contributor

Thanks. I've been experimenting with "nofollow", among other things. There's some evidence that googlebot doesn't always honor it (and of course, there are other bots that don't honor any rules). But it definitely shouldn't hurt, so please leave it in place, thank you!

@ErykKul
Copy link
Collaborator Author

ErykKul commented Jun 16, 2023

The rel="nofollow" attribute does not impact the user experience. I think the engines should not crawl the filters. From what I understand, Yahoo would still crawl the link "nofollow", other engines would not...

@landreev
Copy link
Contributor

the filter doesn't appear right away when a draft is created from a published dataset

I haven't verified yet, but I would guess that this has always been the case.

@kcondon
Copy link
Contributor

kcondon commented Jun 16, 2023

@ErykKul @landreev I just read a bit on nofollow and saw this: Nofollow for internal links. You shouldn’t use nofollow for internal links. If you don’t want a certain page to be crawled or indexed, there are better ways to achieve it, such as robots meta tags.

I am probably missing something but it is mainly used for ranking. Is that the same as crawling? I did not see it say it would not crawl, only not pass that link info back for ranking.

@kcondon
Copy link
Contributor

kcondon commented Jun 16, 2023

the filter doesn't appear right away when a draft is created from a published dataset

I haven't verified yet, but I would guess that this has always been the case.

I tested on demo, it works there.

@kcondon
Copy link
Contributor

kcondon commented Jun 16, 2023

OK, I'm good to merge, fine with trying nofollow as well. Can always change if it doesn't work?

@kcondon kcondon merged commit 1d3f6ed into IQSS:develop Jun 16, 2023
@landreev
Copy link
Contributor

Seeing how publishing has always been asynchronous - not just since #9558 - I would be very surprised if there were a consistent change in this behavior since then. So my money would still be on some combination of timing and coincidence, as an explanation of the observed behavior on demo... but I don't have time to test this hypothesis heavily, so not a large sum of money. 😄

@landreev
Copy link
Contributor

Trying to read on googlebot rules is a rabbit hole... But I am with Eryk in that I don't think it can hurt.

@kcondon
Copy link
Contributor

kcondon commented Jun 16, 2023

I'm happy to take your money ;) Yeah, also saw comments that google now treats nofollow as a hint rather than as a rule.

@pdurbin pdurbin added this to the 5.14 milestone Jun 16, 2023
@coveralls
Copy link

coveralls commented Jun 17, 2023

Coverage Status

coverage: 20.415% (-0.001%) from 20.416% when pulling 1997a44 on ErykKul:9647_bugfix_facets_on_files_page into 49f42ba on IQSS:develop.

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.

Filters for files in dataset page are gone

6 participants