Skip to content
This repository was archived by the owner on Oct 29, 2020. It is now read-only.

Adds campaign run nid to signups and reportbacks#6011

Merged
itsjoekent merged 4 commits intoDoSomethingArchive:devfrom
itsjoekent-archive:6008
Jan 15, 2016
Merged

Adds campaign run nid to signups and reportbacks#6011
itsjoekent merged 4 commits intoDoSomethingArchive:devfrom
itsjoekent-archive:6008

Conversation

@itsjoekent
Copy link
Contributor

What's this PR do?

Adds campaign run nid to signups and reportbacks
(Did not touch the API)

How should this be manually tested?

Is the campaign run NID filled out in signups/reportbacks when you do those respective things?

What are the relevant tickets?

Fixes #6008

Copy link
Contributor

Choose a reason for hiding this comment

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

It's better to name this $language_code to avoid confusion, as $language is global variable.

@itsjoekent
Copy link
Contributor Author

@sergii-tkachenko fixed!

Copy link
Contributor

Choose a reason for hiding this comment

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

What will happen if the field hasn't been translated to the language?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh wait, its the campaign run that gets translated for each, not the field in the campaign? got confused, sorryz

so this should be the nodes source lang, not based on the user lang?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hm. Depends on what do you want to achieve 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.

I'm getting the campaigns current NID.

now that I've rethought about it, I think the procedure is this

campaign blah blah
source lang -> campaign run

campaign run
en --> end date, title, etc
global --> etc etc
etc

@itsjoekent
Copy link
Contributor Author

@sergii-tkachenko ok, i think its good now. changed $language_code to just be $campaign->language

@sergiitk
Copy link
Contributor

sergiitk commented Jan 8, 2016

Are you sure this behaves as expected? Could you please create a campaign in 2 translations and check if $campaign->language changes as we think it is. Also, what if campaign run isn't translated to requested language?

@itsjoekent
Copy link
Contributor Author

blocked on this until a decision is made regarding the biz logic

Copy link
Contributor

Choose a reason for hiding this comment

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

You could potentially use this the field extractor function?

dosomething_helpers_extract_field_data($campaign->field_current_run, $campaign->language);

@weerd
Copy link
Contributor

weerd commented Jan 15, 2016

@deadlybutter @angaither I pulled down this branch and merged it with the work I'm doing. However, I'm already making a bunch of changes which include the code in this PR. I might recommend closing this one and waiting for the upcoming PR to avoid a slew of merge conflicts? It's not a huge deal if there are conflicts, because we can fix them, but figured I'd mention it :)

@angaither
Copy link
Contributor

@weerd I would rather add this in and change it later. I really want to keep the pull requests as small as possible. What did you change here?

I'm also ok with merging this in now if that makes things easier

@weerd
Copy link
Contributor

weerd commented Jan 15, 2016

Had to move the variables around cause they're needed earlier in the function, and a couple other things. Yeah, can we merge this now and I can just rebase the shiz and fix any conflicts :)

Just wasn't sure how long we'd need to hold off on this being merged. But if we can merge now that'd be awesome.

@weerd
Copy link
Contributor

weerd commented Jan 15, 2016

@deadlybutter 👍

itsjoekent pushed a commit that referenced this pull request Jan 15, 2016
Adds campaign run nid to signups and reportbacks
@itsjoekent itsjoekent merged commit d550269 into DoSomethingArchive:dev Jan 15, 2016
@itsjoekent
Copy link
Contributor Author

💥

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants